-
Notifications
You must be signed in to change notification settings - Fork 31
New localstack_host format #45
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
Conversation
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.
Thanks for tackling this @simonrw ! Can you please also add an entry to CHANGELOG.md
, and bump the version in setup.cfg
, so we can release this right away.. 👍
As I mentioned, I think we could use the opportunity to get rid of the service port mapping altogether. This would make our lives easier, as we don't need to manually update the mapping when introducing new services! (which we currently still need to do..)
If we want to keep the changes minimal, we could also tackle this post v2, though (removing USE_LEGACY_PORTS
should have no negative side-effects (as it is anyway no longer working with latest LocalStack), and could be done any time, even in a patch release).
"meteringmarketplace": "{proto}://{host}:4644", | ||
"transcribe": "{proto}://{host}:4566", | ||
"mq": "{proto}://{host}:4566" | ||
_service_ports: Dict[str, int] = { |
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.
I'm inclined to remove this mapping altogether, as it has been painful and is no longer in use. This dates back to the 0.11.x
era (before we even introduced the edge port :) ), and is not working anymore anyways. v2 could be a good opportunity to get rid of USE_LEGACY_PORTS
! 🧹
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.
Let's leave it in for now and revisit later, just in case 👍
This format includes the port number, so the service endpoints have been re-written as service ports, since we can combine that with the LOCALSTACK_HOST variable.
bd75e8c
to
d6e16fa
Compare
We are taking it from `LOCALSTACK_HOST`
The configuration for the main LocalStack project is changing:
We are deprecating
HOSTNAME_EXTERNAL
andLOCALSTACK_HOSTNAME
and in their place usingLOCALSTACK_HOST
with the format<hostname>:<port>
. Since this library is used as part of LocalStack, we need to change the format of theLOCALSTACK_HOST
variable here to match.In addition we document the configuration as suggested in issue #26 and PR #44.
To complement localstack/localstack#7893