Conversation
whummer
left a comment
There was a problem hiding this comment.
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.
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.
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_EXTERNALandLOCALSTACK_HOSTNAMEand in their place usingLOCALSTACK_HOSTwith the format<hostname>:<port>. Since this library is used as part of LocalStack, we need to change the format of theLOCALSTACK_HOSTvariable here to match.In addition we document the configuration as suggested in issue #26 and PR #44.
To complement localstack/localstack#7893