-
Notifications
You must be signed in to change notification settings - Fork 587
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(storage): bump opendal to v0.49.0 #18266
Conversation
Is there any special reason to bump? I think every time we bump OpenDAL we need to do a lot of testing in aws, gcp and azure environments. |
No special purpose 🥵 . Previously I suspected some bug in v0.47 opendal, but actually not. However, I already have finished the coding part, so just create a PR by the way. |
Thanks for the efforts first🙌. |
There is no technical blocker of upgrading OpenDAL to the latest version if we think the latest version is also stable. We just need to run all the tests needed to make sure we are confident about this version before merging. To reduce risks, I suggest we don't bump OpenDAL to latest in 2.0 given that 0.47 is the version we have proved working with several rounds of bug fixing and testing. I think we can certify OpenDAL 0.49 with the same tests we did for 0.47 before merging this PR to main, without blocking release 2.0. @wcy-fdu can decide the schedule because you are more familiar with OpenDAL than us. |
Let's first run all the tests based on this pr, and merge this pr into main if all pass. I will run test based on this PR later, so just hold for a while. |
src/object_store/src/object/opendal_engine/opendal_object_store.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
license-eye has totally checked 5396 files.
Valid | Invalid | Ignored | Fixed |
---|---|---|---|
2296 | 1 | 3099 | 0 |
Click to see the invalid file list
- src/connector/src/parser/protobuf/recursive.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.