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

Please provide a data structure for common S3 information for modular libraries #1108

Closed
1 of 2 tasks
joshtriplett opened this issue Mar 22, 2024 · 2 comments
Closed
1 of 2 tasks
Labels
feature-request A feature should be added or improved.

Comments

@joshtriplett
Copy link
Contributor

Describe the feature

When building a modular library that uses S3 as a backend, there are several things the library needs:

  • An aws_sdk_s3::Client to make requests with.
  • A bucket to use (for libraries that keep data in one bucket)
  • An optional key prefix (to make it easy for multiple libraries to share one bucket)
  • An optional expected_bucket_owner, as a security cross-check

It'd be nice to be able to put all of these together in a common data structure and pass that data structure to a library. Then, the library could make get_object or put_object requests (for instance) with a default for the bucket and expected_bucket_owner fields and a default prefix on the key.

Use Case

I'd like to build modular libraries, and avoid having to pass all four of these fields separately to the fn new for the library's object(s) and handle them separately for each method.

Proposed Solution

I can see 3 reasonable ways to do this:

Option 1 (simplest to implement):

  • Add a new structure opaquely wrapping the three fields other than the S3 client.
  • Generate methods on all of the fluent builders that accept the structure, and set the corresponding values. For the key, remember the prefix and then when calling key prepend the prefix before setting the key.

Downsides of this approach are that the library has to remember to pass the structure to every method call, and that the library has to accept both an S3 client and the structure.

Option 2 (simplest to use, slight overhead):

  • Add fields for default values to aws_sdk_s3::Client (outside of the Arc, so that they're not shared among Client instances).
  • Modify the generated methods such that they use those values as defaults, and prepend the key prefix when setting the key.

Option 3 (reasonably simple to use, no overhead if not used, requires much more work in code generation):

  • Add a new structure wrapping aws_sdk_s3::Client and adding these fields.
  • Give the structure methods corresponding to each method of Client. When calling those methods, set the defaults.
  • This would require some additional support in the fluent builders for setting a key prefix.

Of these three, I would personally recommend Option 2 or option 1, in that order of preference.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

A note for the community

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue, please leave a comment
@joshtriplett joshtriplett added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 22, 2024
@rcoh
Copy link
Contributor

rcoh commented Mar 25, 2024

You can do this with an interceptor that you add to the client. A small amount of manual work is required for each operation you'd like to support to extract the bucket and the key. A sketch below:

struct S3DefaultConfigIntercetpor {
  default_bucket: String,
  key_prefix: String
}

impl Intercept for S3DefaultConfigInterceptor {
   fn modify_before_serialization(...) { 
     if let Some((bucket, key)) = bucket_key!(context.input_mut(), List, Of, Operations, You, Want, To Support) { /* mutate bucket and key */ }
   }
}
// approximately, I did not test this
macro_rules! bucket_key {
    ($input: expr, $($operation:ident),*) => {{
        let input: &mut dyn Any = &mut $input;
        $(
            if let Some(inp) = input.downcast_mut::<$operation>() {
                Some((&mut inp.bucket, &mut inp.key))
            } else
        )*
        {
            None
        }
    }};
}

We are very unlikely to ever support this in the generated client, because, generally, we avoid adding new custom behavior to generated code.

@ysaito1001 ysaito1001 removed the needs-triage This issue or PR still needs to be triaged. label Mar 26, 2024
@jmklix jmklix closed this as completed Mar 29, 2024
Copy link

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

4 participants