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

Provide client config property for region provider #1478

Open
1 task done
ianbotsf opened this issue Dec 13, 2024 · 3 comments
Open
1 task done

Provide client config property for region provider #1478

ianbotsf opened this issue Dec 13, 2024 · 3 comments
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue

Comments

@ianbotsf
Copy link
Contributor

Describe the feature

Currently, SDK client config has a region property which is a non-defered static string. Internally, the SDK uses RegionProvider and DefaultRegionProviderChain to lazily infer the region when one is not explicitly provided by the caller. This complicates dynamic region selection for callers who use their own configuration lookup mechanisms or want to use custom provider chains.

Is your feature request related to a problem?

The lack of a regionProvider property complicates dynamic region selection for callers who use their own configuration lookup mechanisms or want to use custom provider chains. Users must manage their own chain independently and then call getRegion() to pass to the client's region property:

val myRegionProvider = RegionProviderChain(src1, src2, src3, ...)

val s3 = S3Client.fromEnvironment {
    region = myRegionProvider.getRegion()
}

Proposed Solution

Expose a regionProvider client config property alongside region. When set, it should replace the default region provider chain used when a static region is not specified:

val myRegionProvider = RegionProviderChain(src1, src2, src3, ...)

val s3 = S3Client.fromEnvironment {
    regionProvider = myRegionProvider 
}

If a static region is specified, the value of regionProvider will not be used:

val myRegionProvider = ...

val s3 = S3Client.fromEnvironment {
    regionProvider = myRegionProvider // Ignored since `region` is also set
    region = "moon-east-1"
}

Describe alternative solutions or features you've considered

The existing region property presently allows using providers indirectly but it's not very ergonomic and differs from how credentials are resolved.

Acknowledge

  • I may be able to implement this feature request

AWS SDK for Kotlin version

1.3.93

Platform (JVM/JS/Native)

(any)

Operating system and version

(any)

@ianbotsf ianbotsf added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Dec 13, 2024
@cloudshiftchris
Copy link

Is this the right thing to do?

   regionProvider = myRegionProvider // Ignored since `region` is also set
    region = "moon-east-1"

Alternatively this could error out - but that may be surprising in more complicated cases where the configuration is composed (not always two lines of code together), and perhaps hard to resolve.

Also an option is to flip the resolution, on the premise that a RegionProvider may be more relevant than static configuration (but not always...).

Are there other *Providers in the SDK that have perhaps already addressed this?

I don't have a strong opinion or specific requirements here - whatever you decide will be fine, so long as the "conflict resolution" behaviour is clearly documented.

@ianbotsf
Copy link
Contributor Author

My instinct is to bias towards the most specific option (i.e., region) rather than a less specific option (i.e., regionProvider). This matches the current behavior of endpointUrl and endpointProviderendpointUrl wins if both are set. We'll discuss internally in more depth to verify that's the best approach but if you have further thoughts about one approach vs the other we'd love to take those into account. 🙂 And of course whichever option we select, we'll need to update/clarify our documentation.

If we do stick with the proposed region > regionProvider approach, it might be sensible to log a warning when both are set. This would probably apply to endpointUrl > endpointProvider too.

@cloudshiftchris
Copy link

Agree on the "most specific" approach; good to know that endpoint* already has that behaviour.

Like the idea of logging a warning - silently ignoring things is a 💣 that we'd all prefer to avoid!

Possibly over engineering - add a "configuration resolution mode" or some such settings, defaults to Warn, could also be Error. I'd expect this would get rarely used, though, perhaps not worth the effort.

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. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

2 participants