Configured endpoint URL resolution #2973
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## develop #2973 +/- ##
===========================================
+ Coverage 93.32% 93.36% +0.04%
===========================================
Files 65 65
Lines 13764 13840 +76
===========================================
+ Hits 12845 12922 +77
+ Misses 919 918 -1
☔ View full report in Codecov by Sentry. |
kyleknap
reviewed
Jun 20, 2023
kdaily
force-pushed
the
configured-endpoint-urls
branch
from
e43091c to
f2d4384
Compare
nateprewitt
requested changes
Jun 20, 2023
|
Will this work for setting the S3 endpoint url? |
|
when will you merge it? |
kdaily
force-pushed
the
configured-endpoint-urls
branch
2 times, most recently
from
63cb45d to
9ba6fdd
Compare
When deep copying the config value store, override values were not preserved.
Instead of deep copying, use a shallow copy of the config value store when using it for creating a new client from a session. Only use a deep copy for updating the section provider. Update behavior of the smart defaults update functions to use a copy of the existing config providers in all cases. Update the logic for determining which new provider to create. Move unit tests for config provider smart defaults to functional tests and rework them to better test behavior. A mocked defaults data file is used to confirm that the correct values are loaded to protect against changes to the live data file.
Adds a config provider to resolve the endpoint URL provided in an environment variable or shared configuration file. It resolves the endpoint in the following manner: 1. The value provided through the `endpoint_url` parameter provided to the client constructor. 2. The value provided by a service-specific environment variable. 3. The value provided by the global endpoint environment variable (`AWS_ENDPOINT_URL`). 4. The value provided by a service-specific parameter from a services definition section in the shared configuration file. 5. The value provided by the global parameter from a services definition section in the shared configuration file. 6. The value resolved when no configured endpoint URL is provided. The endpoint config provider uses the client name (name used to instantiate a client object) for construction and add to the config value store. This uses multiple lookups to handle service name changes for backwards compatibility.
The tests use a data file to load tests that enumerate the ways that the endpoint URL can be defined and asserts that the correct endpoint is used in a request and that it is ignored correctly if the appropriate shared config file property or environment variable is supplied. They also assert that the correct config section name and environment variable name are used to configure and override the endpoint URL for every AWS service.
Add a debug statement when specifically ignoring the configured endpoint URL from the shared configuration file or an environment variable. This will not be logged if the endpoint URL is set through the client constructor.
Simplify docstring and fix typos.
Add tests to check that setting the client creation parameter to ignore the configured endpoint URLs is respected.
kyleknap
reviewed
Jul 5, 2023
| ) | ||
|
|
||
|
|
||
| def _known_service_names_and_models(): |
There was a problem hiding this comment.
Let's rename this to _known_service_names_and_ids() to be consistent with the return value
GitHub Actions was consistently failing on Windows instances with this test. Likely cause was loading the large model files into memory, which persisted until testing was completed. Since the rest of the model is not needed, only return the service name and ID.
kdaily
force-pushed
the
configured-endpoint-urls
branch
from
e55ca6a to
2fa2224
Compare
This was referenced Jul 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the proposal for configuring endpoints per service:
aws/aws-sdk#230
Adds a config provider to resolve the endpoint URL provided in an environment variable or shared configuration file. It resolves the endpoint in the following manner:
endpoint_urlparameter provided to the client constructor.AWS_ENDPOINT_URL).The endpoint config provider uses the client name (name used to instantiate a client object) for construction and add to the config value store. This uses multiple client/service name lookups to handle service name changes for backwards compatibility.