Skip to content

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Mar 21, 2023

The configuration for the main LocalStack project is changing:

We are deprecating HOSTNAME_EXTERNAL and LOCALSTACK_HOSTNAME and in their place using LOCALSTACK_HOST with the format <hostname>:<port>. Since this library is used as part of LocalStack, we need to change the format of the LOCALSTACK_HOST variable here to match.

In addition we document the configuration as suggested in issue #26 and PR #44.

To complement localstack/localstack#7893

@simonrw simonrw self-assigned this Mar 21, 2023
@simonrw simonrw marked this pull request as ready for review March 22, 2023 16:53
@simonrw simonrw requested a review from whummer March 22, 2023 16:59
Copy link
Member

@whummer whummer left a 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] = {
Copy link
Member

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! 🧹

Copy link
Contributor Author

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 👍

simonrw added 2 commits March 22, 2023 19:28
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.
@simonrw simonrw force-pushed the new-localstack-host-format branch from bd75e8c to d6e16fa Compare March 22, 2023 19:29
@simonrw simonrw merged commit 14aadb3 into master Mar 23, 2023
@simonrw simonrw deleted the new-localstack-host-format branch March 23, 2023 12:04
simonrw added a commit to localstack/localstack that referenced this pull request Mar 23, 2023
simonrw added a commit to localstack/localstack that referenced this pull request Mar 23, 2023
simonrw added a commit to localstack/localstack that referenced this pull request Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants