GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — ASF/CloudWatch: add support for multi-protocols by bentsku · Pull Request #13161 · localstack/localstack
This repository was archived by the owner on Mar 23, 2026. It is now read-only.
Conversation
|
S3 Image Test Results (AMD64 / ARM64) 2 files ±0 2 suites ±0 7m 45s ⏱️ -19s Results for commit 0d8f406. ± Comparison against base commit f188c62. ♻️ This comment has been updated with latest results. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Test Results - Preflight, Unit22 277 tests +82 20 539 ✅ +82 17m 14s ⏱️ + 2m 12s Results for commit 0d8f406. ± Comparison against base commit f188c62. ♻️ This comment has been updated with latest results. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Test Results (amd64) - Acceptance7 tests ±0 5 ✅ ±0 3m 19s ⏱️ -1s Results for commit 0d8f406. ± Comparison against base commit f188c62. ♻️ This comment has been updated with latest results. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Test Results - Alternative Providers66 tests 38 ✅ 3m 32s ⏱️ Results for commit 0d8f406. ♻️ This comment has been updated with latest results. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 36m 38s ⏱️ Results for commit 0d8f406. ♻️ This comment has been updated with latest results. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
LocalStack Community integration with Pro 2 files 2 suites 1h 58m 53s ⏱️ Results for commit 0d8f406. ♻️ This comment has been updated with latest results. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
00ccd61 to
0f27df7
Compare
51871cc to
61416ea
Compare
0f27df7 to
7e3922b
Compare
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Absolutely awesome! The final piece of the puzzle for #13028! It already performs the modifications on the specs which are about to come in the future release (according to the preview build) 🤩
It continues to surprise me how hard "time" can be (there was quite some unexpected stuff with the tz handling for cloudwatch in this PR) 😅, and the moto fallback handling is an interesting case as well. 💯
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| """ | ||
| This function is taken from botocore Client._convert_to_request_dict, but we are overriding the serializer | ||
| Botocore does not expose a way to create a client with a specific protocol, but we need this functionality | ||
| to support multi-protocols. | ||
| """ |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
thought: Just keeping note of what we discussed yesterday. I think this is an interesting example where a feature like this (manual protocol selection / changing the precedence for a request) could be interesting in botocore. However, we have been using botocore internals here before, so for now I think this approach is just fine. 💯
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yes, definitely agree 💯 hopefully we get to configure that on the client level, for now I'll use patching to run the full test suite 👍
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| @markers.aws.validated | ||
| @pytest.mark.parametrize("protocol", ["json", "smithy-rpc-v2-cbor", "query"]) | ||
| @markers.snapshot.skip_snapshot_verify( | ||
| paths=[ | ||
| # LogAlarms is not defined in the Boto specs anywhere, but is returned in raw responses (deprecated?) | ||
| "$.describe-alarms..LogAlarms", | ||
| ] | ||
| ) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
praise: 🥳 This is the test which actually runs against all 3 protocols! 🤩
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
praise: This is basically patching the existing botocore specs with what the preview builds of botocore for this feature are doing. 💯
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| action=context.operation.name, | ||
| parameters=service_request, | ||
| region=context.region, | ||
| protocol=context.protocol, |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
thought (non-blocking): That's an interesting one - as you described in your PR description - as it won't work for CBOR protocols with moto just yet. For now I definitely agree that we should not over-complicate this here, especially since:
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's true. It will also be the case for all regular call_moto, as we don't re-serialize the request there.
I'd also be a bit worried about changing the overall format of the request to be able to pass it to 3rd party backend, as it could lead to unexpected behavior as well (meaning going rpc-v2-cbor -> json -> parse json back -> serialize rpc-v2-cbor)
But good to be aware of it 👍
Sorry, something went wrong.
There was an error while loading. Please reload this page.
e28e8fe to
0d8f406
Compare
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.