Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trackings issues of the first release of OpenDAL C binding #2321

Closed
3 of 6 tasks
Xuanwo opened this issue May 25, 2023 · 44 comments
Closed
3 of 6 tasks

Trackings issues of the first release of OpenDAL C binding #2321

Xuanwo opened this issue May 25, 2023 · 44 comments

Comments

@Xuanwo
Copy link
Member

Xuanwo commented May 25, 2023

OpenDAL C binding is get started but there are some tasks we need to address for our first release.

Tasks

  • Implement operator builder
  • Implement all behavior tests
  • OpenDAL C docs (maybe doxygen) @xyjixyjixyji
  • OpenDAL C examples
  • Workflows for linux/windows/macos release @Xuanwo
    • Maybe deb/rpm for ubuntu/centos?
@Xuanwo
Copy link
Member Author

Xuanwo commented May 25, 2023

Hey @Ji-Xinyou, we previously worked on implementing a code generator for the operator builder. However, I realized that it's not a good idea. It would be more beneficial for us to begin with Operator::via_map instead.

Do you still interested in implement operator_new with a map in c binding?

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented May 25, 2023

Hey @Ji-Xinyou, we previously worked on implementing a code generator for the operator builder. However, I realized that it's not a good idea. It would be more beneficial for us to begin with Operator::via_map instead.

Do you still interested in implement operator_new with a map in c binding?

Of course. A map way is certainly acceptable, but I think a builder based way is also needed. What do you think

@Xuanwo
Copy link
Member Author

Xuanwo commented May 25, 2023

but I think a builder based way is also needed

I agree with this too. However, since we've spent a significant amount of time on it already, perhaps we can begin by offering users a functional C binding first.

@jiaoew1991
Copy link
Contributor

Looking forward to the improvement of c binding ability.

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented May 26, 2023

I think several examples for typical use cases are needed for the release, since we do not have any official documentation for C binding yet.

@Xuanwo
Copy link
Member Author

Xuanwo commented May 26, 2023

I think several examples for typical use cases are needed for the release, since we do not have any official documentation for C binding yet.

Seems C doesn't have docs genenator like rustdoc, we can add them in website directly.

@xyjixyjixyji
Copy link
Contributor

Seems C doesn't have docs genenator like rustdoc, we can add them in website directly.

The doxygen would be a way to generate docs, we could use that.

@Xuanwo
Copy link
Member Author

Xuanwo commented May 26, 2023

The doxygen would be a way to generate docs, we could use that.

I'm a bit afraid of the complexity of doxygen, but it's still okay to use. Thanks for remind!

@xyjixyjixyji
Copy link
Contributor

As for the behavior tests, what tests are expected. So far I understand this as implement all behavior tests in Rust core for the C binding (such as write_test etc.). Please notify me if there is any misunderstanding.

@suyanhanx
Copy link
Member

suyanhanx commented May 27, 2023

As for the behavior tests, what tests are expected. So far I understand this as implement all behavior tests in Rust core for the C binding (such as write_test etc.). Please notify me if there is any misunderstanding.

Please refer to bindings/tests/features/binding.feature for now.

@xyjixyjixyji
Copy link
Contributor

Please refer to bindings/test for now.

That has already been implemented in bindings/c/tests/bdd.cpp.

@Xuanwo
Copy link
Member Author

Xuanwo commented May 27, 2023

I think we should align with our features. For example:

https://github.com/apache/incubator-opendal/blob/8797a2424bdc294e6936e49c52e5f31abd4ed002/bindings/c/tests/bdd.cpp#L68-L69

We should not call opendal_operator_blocking_write in SetUp.

Others LGTM, we can start working on some examples and prepare for release.

@xyjixyjixyji
Copy link
Contributor

I think we should align with our features. For example:

https://github.com/apache/incubator-opendal/blob/8797a2424bdc294e6936e49c52e5f31abd4ed002/bindings/c/tests/bdd.cpp#L68-L69

We should not call opendal_operator_blocking_write in SetUp.

Others LGTM, we can start working on some examples and prepare for release.

I will modify it to align with the bdd feature. I did that because all the tests need a write before hand lol.

@Xuanwo
Copy link
Member Author

Xuanwo commented May 27, 2023

Hi, @Ji-Xinyou, I have updated our tasks list, please take a look. Thanks!

@xyjixyjixyji
Copy link
Contributor

Just a idea. What about we inline the documentation in the comment doc of our C binding code. User can directly use cargo doc to see the documentation.

@suyanhanx
Copy link
Member

But you still need to provide a way to view it online. I think it's better to provide the documentation with the familiar look of the C ecosystem.

@xyjixyjixyji
Copy link
Contributor

But you still need to provide a way to view it online. I think it's better to provide the documentation with the familiar look of the C ecosystem.

Then we should use doxygen then. I will look into this on how to better integrate the Rust comments with doxygen-style comments.

@Xuanwo
Copy link
Member Author

Xuanwo commented May 28, 2023

Just a idea. What about we inline the documentation in the comment doc of our C binding code. User can directly use cargo doc to see the documentation.

I don't like this idea. Every binding should use their own way. For example, don't require C/CPP users to install or know about cargo.

@Xuanwo
Copy link
Member Author

Xuanwo commented May 28, 2023

If doxygen is too complicated to use, we can write API docs in website directly.

@xyjixyjixyji
Copy link
Contributor

If doxygen is too complicated to use, we can write API docs in website directly.

I will try doxygen first. If that's not working, we write docs in the website directly.

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented May 29, 2023

Great news, the doxygen is working well IMO 😆

One concern when rewriting the doc, how does user know the suitable key-value pairs to passed into the opendal_operator_new. It seems that the users currently have to look it up rather in the from_map implementation for each Backend or in here. I think here would be great idea.

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented Jun 4, 2023

I would appreciaite if there are some opinions. As for the example part, which of the following do we need.

  • An example with classical usage on many APIs, such as stat, is_exist, read and write
  • A small example with only read and write, and multiple others showing some other APIs

I personally prefer the second one since the API will grows and putting them altogether is not a good practice IMO.

@Xuanwo
Copy link
Member Author

Xuanwo commented Jun 4, 2023

  • A small example with only read and write, and multiple others showing some other APIs

LGTM! Most users will use OpenDAL for specific purposes, such as reading data from a remote location, writing data to a remote location, or listing files. We can provide examples for them. Maybe in website docs directly like services?

We can also share the markdown files between website and code too if needed. But for simple, we can start by in website first.

@xyjixyjixyji
Copy link
Contributor

We can also share the markdown files between website and code too if needed. But for simple, we can start by in website first.

What abt this. We setup a most common read + write example in both ./bindings/c/examples and the website. And put other examples in ./bindings/c/examples only and mention that in the website documentation.

@suyanhanx
Copy link
Member

Maybe in website docs directly like services?

We can introduce our public API and give examples like API reference.

@Xuanwo
Copy link
Member Author

Xuanwo commented Jun 5, 2023

What abt this. We setup a most common read + write example in both ./bindings/c/examples and the website. And put other examples in ./bindings/c/examples only and mention that in the website documentation.

LGTM, let's get work started.

@jiaoew1991
Copy link
Contributor

Request an "opendal_operator_blocking_list" interface in C binding 😄.

@xyjixyjixyji
Copy link
Contributor

Request an "opendal_operator_blocking_list" interface in C binding 😄.

I will work on that ASAP 😆

@Xuanwo
Copy link
Member Author

Xuanwo commented Jun 7, 2023

I will work on that ASAP

Let's track this in a new issue instead. I believe it not a block of our first release.

@jiaoew1991
Copy link
Contributor

Request an "opendal_operator_blocking_list" interface in C binding 😄.

I will work on that ASAP 😆

Hi @Ji-Xinyou How's this going? 😊

@xyjixyjixyji
Copy link
Contributor

Request an "opendal_operator_blocking_list" interface in C binding 😄.

I will work on that ASAP 😆

Hi @Ji-Xinyou How's this going? 😊

I expect this to be done today or tomorrow. Sorry for being late. I have been really busy recently.

@xyjixyjixyji
Copy link
Contributor

@jiaoew1991 See #2448 , just wait this to be merged ❤️

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented Oct 25, 2023

I think we should start considering the release for C binding.

From my side, I will check if more examples are needed for the C binding.

The workflow should be running ok on MacOS and Linux now, are they running fine on Windows?

For the release, are we releasing for Windows/MacOS as well?

@Xuanwo
Copy link
Member Author

Xuanwo commented Oct 25, 2023

I think we should start considering the release for C binding.

We're on track to graduate from ASF Incubator. How about we start this project after graduation?

Before we start our C binding release, there are some thing we can start now:

  • Add guide for C binding.
  • Implement test framework for c binding (like nodejs and python)

@xyjixyjixyji
Copy link
Contributor

I think we should start considering the release for C binding.

We're on track to graduate from ASF Incubator. How about we start this project after graduation?

I think so. That's 100% with higher priority.

@Xuanwo
Copy link
Member Author

Xuanwo commented Oct 25, 2023

I think so. That's 100% with higher priority.

Thanks! I'm guessing we can add test framework first to make sure C binding works on all our services.

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented Aug 13, 2024

Now we have testings and examples for opendal C bindings, i would like to achieve this release since i have some bandwidth now.

Except for releasing, what features and requirements do we need for the first release? What i can think of is async support, hyper has a really good implementation reference basically we can copy paste....., its licence allows free of use.

For releasing, since the bindings functionality grows with the core, the semver thing still need some discussion. I am thinking for each update release in core, we release C with a PATCH version; for each important feature implementation, we release with a MINOR; for each breaking change, we release with a MAJOR.

\cc @Xuanwo

@Xuanwo
Copy link
Member Author

Xuanwo commented Aug 13, 2024

Now we have testings and examples for opendal C bindings, i would like to achieve this release since i have some bandwidth now.

In fact, we have released C multiple times as a tarball, such as https://dlcdn.apache.org/opendal/0.48.0/. I believe we can continue to release it this way.

Additionally, does the user want libopendal_c.so or something similar?

For releasing, since the bindings functionality grows with the core, the semver thing still need some discussion. I am thinking for each update release in core, we release C with a PATCH version; for each important feature implementation, we release with a MINOR; for each breaking change, we release with a MAJOR.

The current release strategy for the entire OpenDAL project is as follows:

  • No breaking changes: PATCH.
  • Breaking changes: MINOR.

We will introduce MAJOR updates once our core reaches version 1.0.

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented Aug 13, 2024

I think we still need to release it as deb/rpm packages for user to install it easily when they build in a container.

One concern is that releasing through deb/rpm seems like not the way that Apache releases work, so does that mean we release the so in the release should be fine?

The release strategy SGTM.

@Xuanwo
Copy link
Member Author

Xuanwo commented Aug 13, 2024

One concern is that releasing through deb/rpm seems like not the way that Apache releases work, so does that mean we release the so in the release should be fine?

Binary releases like deb/rpm are not part of the Apache release; we provide them solely for user convenience.

Generally, we do not include .so files in the release. Instead, we need to build and upload them to https://apache.jfrog.io/, similar to how Arrow does it: https://arrow.apache.org/install/.

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented Aug 13, 2024

SG, then we keep releasing so in this way should be good. We could implement an async support, make the doc more compelete and then we are good to go?

@Xuanwo
Copy link
Member Author

Xuanwo commented Aug 13, 2024

We could implement an async support, make the doc more compelete and then we are good to go?

We will maintain the current release workflow, ensuring that tasks do not block each other. This allows us to continue adding new features, such as async support and additional docs, simultaneously.

@xyjixyjixyji
Copy link
Contributor

xyjixyjixyji commented Aug 13, 2024

IC, i am mentioning this cuz we don't have an official release on the homepage README 😉 But i guess we don't need an Official release then

@Xuanwo
Copy link
Member Author

Xuanwo commented Aug 13, 2024

IC, i am mentioning this cuz we don't have an official release on the homepage README 😉 But i guess we don't need an Official release then

Yes, we don't need that. We can point to the correct place of our tar and let user know how to build them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants