Minor fixes in edge forwarder and zip utils for dev/host mode #6065
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
whummer
commented
May 15, 2022
| @@ -32,7 +32,6 @@ def test_start_and_stop(self, monkeypatch): | |||
| backend_port, | |||
| update_listener=None, | |||
| quiet=True, | |||
| params={"protocol_version": "HTTP/1.0"}, | |||
There was a problem hiding this comment.
Value currently not being used/processed downstream, so this change does not change the logic itself.
alexrashed
approved these changes
May 16, 2022
There was a problem hiding this comment.
Thanks for the fix! Looks great and actually fixes the issue which would have been addressed with #6059. 🚀
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Minor fixes for issues that surfaced during testing in local dev/host mode:
When using the HTTPS forwarding server on port 443 (using
EDGE_FORWARD_URL), we need to forward the requests using the raw path of the HTTP request (otherwise request parsing fails downstream, e.g., for URL-encoded ARNs passed in request paths). Issue can be replicated, e.g., with this command (after starting LS in Pro host mode):awslocal kafka --endpoint-url https://localhost.localstack.cloud get-bootstrap-brokers --cluster-arn arn:aws:kafka:us-east-1:000000000000:cluster/test. The existingget_raw_path(..)http utility comes in handy here. Added a small test for it as well.Prefer using the native
unzipcommand (if available) over the built-in Pythonzipfileutils. Some versions of Node.js / Serverless are generating zip files with invalid/incorrect CRC codes (e.g., here) - the files extract fine withunzip(potentially generating some warnings), but the unzipping fails with Python utils (empty files being created on disk). In future, we could think about introducing astrictmode for theunziputils to enforce CRC codes, if needed.The changes above should not impact the current logic inside the Docker container (only relevant for host mode).
Additionally, the PR contains a few minor cleanups:
paramsargument for thestart_proxy(..)utility - resolves aTODO: not being usedcommenttime.sleep(..)calls in the tests