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

feat(bin/ofs): introduce integrations/cloudfilter for ofs #4935

Merged
merged 6 commits into from
Aug 19, 2024

Conversation

ho-229
Copy link
Contributor

@ho-229 ho-229 commented Jul 28, 2024

What changes are included in this PR?

Introduce integrations/cloudfilter for ofs.

@Xuanwo
Copy link
Member

Xuanwo commented Jul 28, 2024

Thank you for this PR. However, I'm not confident that integrations/cloudfilter is ready for use. Do you think we need to enhance integrations/cloudfilter further by adding documentation, comments, examples, and tests?

@ho-229
Copy link
Contributor Author

ho-229 commented Jul 28, 2024

Hello @Xuanwo and @oowl, PTAL.

@oowl
Copy link
Member

oowl commented Jul 29, 2024

We need to set up a new Windows action environment to run a Windows test for the new cloudfilter fs test. Besides, https://github.com/ho-229/cloud-filter-rs seems does not have any test. Maybe we need to complete integration or unit tests first.

@ho-229 ho-229 marked this pull request as draft July 29, 2024 06:27
@ho-229
Copy link
Contributor Author

ho-229 commented Aug 3, 2024

Hello @oowl, I have been add some tests for https://github.com/ho-229/cloud-filter-rs, and I think maybe we can add tests for integration/cloudfilter in next PR.

@ho-229 ho-229 marked this pull request as ready for review August 15, 2024 13:38
@ho-229
Copy link
Contributor Author

ho-229 commented Aug 15, 2024

Hi @Xuanwo and @oowl , we have completed the improvements in integrations/cloudfilter. Should we proceed?

@oowl
Copy link
Member

oowl commented Aug 17, 2024

It seems the cloudfilter repo has some basic tests for reading and writing, but we all know the file system test case is so complex, That it is not enough. But can we try to merge the current PR first? what do you think? @Xuanwo

@Xuanwo
Copy link
Member

Xuanwo commented Aug 17, 2024

It seems the cloudfilter repo has some basic tests for reading and writing, but we all know the file system test case is so complex, That it is not enough. But can we try to merge the current PR first? what do you think? @Xuanwo

Makes sense to me. We can move forward.

@ho-229
Copy link
Contributor Author

ho-229 commented Aug 17, 2024

Hi @oowl , I think cloudfilter is not a filesystem, we are currently focusing on reading files from remote.

@oowl
Copy link
Member

oowl commented Aug 17, 2024

Hi @oowl , I think cloudfilter is not a filesystem, we are currently focusing on reading files from remote.

Do we have any limitations for CRUD action implementation?

@ho-229
Copy link
Contributor Author

ho-229 commented Aug 17, 2024

Hi @oowl , I think cloudfilter is not a filesystem, we are currently focusing on reading files from remote.

Do we have any limitations for CRUD action implementation?

We now support fetching data and the path tree from a remote source and making changes locally. However, these changes cannot be synced to the remote source

bin/ofs/tests/common/mod.rs Show resolved Hide resolved
@oowl
Copy link
Member

oowl commented Aug 19, 2024

Hi @oowl , I think cloudfilter is not a filesystem, we are currently focusing on reading files from remote.

Do we have any limitations for CRUD action implementation?

We now support fetching data and the path tree from a remote source and making changes locally. However, these changes cannot be synced to the remote source

So from our goal, we not only support fetching data and the path tree from a remote source but also need to support delete or update operations. Do I understand correctly?

@ho-229
Copy link
Contributor Author

ho-229 commented Aug 19, 2024

Hi @oowl , I think cloudfilter is not a filesystem, we are currently focusing on reading files from remote.

Do we have any limitations for CRUD action implementation?

We now support fetching data and the path tree from a remote source and making changes locally. However, these changes cannot be synced to the remote source

So from our goal, we not only support fetching data and the path tree from a remote source but also need to support delete or update operations. Do I understand correctly?

Yes, but I encounter some problems to implement dehydration for cloudfilter_opendal, so I prefer to implement the readonly version first.

@oowl oowl merged commit 362d140 into apache:main Aug 19, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants