-
Notifications
You must be signed in to change notification settings - Fork 364
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
Default configuration S3Client #2923
Comments
@moradology @echeipesh could one of you clarify the ask here? As far as I can tell, it appears that (1) is already implemented in |
@CloudNiner Assertion: changing the default timeout and back-off settings on the client is a very common use case. Being able to provider a client directly to layer/writer/reader covers it partially but is not sufficient. This is actually not much of assertion, we've seen this many times. Opinion: Having a global mutable setting for current client is not desirable long term. Its not standard and potentially dangerous if in client code it gets set in multiple places or out of order. Both of which are possible as projects grow and code modules get combined. Ask: Replace the global state with boring SPI Use cases:
Open question: Should the SPI provide and instance of the client or just the configuration for it ? |
Classes which are summoned via the
ClassLoader
mechanism (as in the SPI pattern) must not have parameters in their constructors. ACassandraLayerReader
and anS3LayerReader
are both instanced through the same method which takes a string and delegates to the appropriate SPI class depending on the pattern found in said string. This is handy, but it elides important details in at least the S3 case:S3Client
objects are complicated and the 'Default Credential Provider' chain won't always be sufficient.Given these two constraints (no parameters and the need to configure in certain cases), we need either
S3ClientBuilder
APIThe text was updated successfully, but these errors were encountered: