-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add Endpoints 2.0 Rulesets #2801
Conversation
@pytest.mark.parametrize("service_name", AVAILABLE_SERVICES) | ||
def test_all_endpoint_tests_exist(service_name): | ||
"""Tests the existence of endpoint-tests-1.json for each service | ||
and verifies that content is present.""" | ||
endpoint_tests_file = os.path.join( | ||
TEST_DATA_DIR, service_name, 'endpoint-tests-1.json' | ||
) | ||
with open(endpoint_tests_file) as f: | ||
data = json.load(f) | ||
assert len(data['testCases']) >= 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test already exists in tests/functional/test_endpoint_rulesets.py:test_all_endpoint_tests_exist
. In my opinion, it should stay there and be removed here because test_endpoint_rulesets.py
includes other tests that use the contents of these files, all of which share a cache to reduce the number of times the endpoint-tests-1.json
files need to be loaded.
It would probably make sense for this PR to also include some changes to the if service_name in ENDPOINT_RESOLUTION_V2_SERVICES or FORCE_ENDPOINT_RESOLUTION_V2 After this PR, most (but not all) of the tests in |
Codecov ReportBase: 93.57% // Head: 93.57% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #2801 +/- ##
========================================
Coverage 93.57% 93.57%
========================================
Files 63 63
Lines 13300 13305 +5
========================================
+ Hits 12445 12450 +5
Misses 855 855
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
01deeaa
to
3654140
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Overview
This PR will add Endpoint Rulesets for all services as a follow up to #2785. These will provide deterministic rulesets for deriving endpoints based on user input, moving away from customized code for evaluating user input to resolve operation destination. This will improve inter-SDK compatibility for how endpoints are resolved, reducing a number of inconsistencies between different language implementations, as well as abstract the logic out of complex Botocore events.
This change will supersede the existing
endpoints.json
that many users are familiar with. To preserve backwards compatibility, we'll detect any deviations in our loadedendpoints.json
file from what was distributed with the source code. If a variation is detected, we will use our old endpoints logic to load any customized endpoints.Botocore Size
With this change, we'll be adding a considerable number of additional models to Botocore. We're aware this is a common pain point for users given the size of Botocore. We've spent time this year to add optional support for gzipped models.
Last week, we rolled out our first usage of this by compressing these rulesets for both S3 and S3Control for our wheel distribution on PyPI. This initial test demonstrated an average space savings of 97.5% for our first two models. We'll be using this to minimize the initial impact of these model additions while we explore compressing the base
service-2.json
models in a future release. This has the potential to see a dramatic decrease in the overall size for Botocore. We'll track future work on this in the initial tracking issue (#2365).