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

refactor: Relax bounds on Writer::{sink, copy} #3027

Merged
merged 1 commit into from
Sep 10, 2023

Conversation

huonw
Copy link
Contributor

@huonw huonw commented Sep 10, 2023

This adjusts the trait bounds on the input stream (for sink) or reader (for copy) for the opendal::Writer type to be less strict, and thus allow more types to be passed in.

In particular, the current implementations of these functions do not use any of the additional "information"/requirements that Send + Sync + Unpin + 'static imply. The 'static bound is particularly restrictive, as it stops one being able to pass in any borrowed data, such as a &mut dyn AsyncRead object or similar. Without these bounds, one can pass in more readers or more streams, more simply.

There is a downside to this change: if these implementations wanted to change in future to use any of these (e.g. a threaded implementation of copy or something), that becomes a breaking change. I would suggest that's okay: any fancy implementation like that would be best offered in addition to these simple ones, exactly because they'll need to impose additional restrictions on their input types.

(Thank you for OpenDAL!)

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@Xuanwo Xuanwo merged commit 3bced97 into apache:main Sep 10, 2023
92 checks passed
@huonw huonw deleted the simplify-writer-bounds branch September 10, 2023 04:37
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.

None yet

2 participants