Skip to content
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

Organize config list #9654

Merged
merged 5 commits into from Nov 15, 2023
Merged

Organize config list #9654

merged 5 commits into from Nov 15, 2023

Conversation

joe4dev
Copy link
Member

@joe4dev joe4dev commented Nov 15, 2023

Motivation

localstack.config.py has become bloated by mixing up deprecated, removed, and current configurations.

HOSTNAME_FROM_LAMBDA is deprecated but still supported.

Changes

This PR primarily improves the organization of the config list and removes a deprecation:

  • Synchronize removal grouping with the docs PR Collect more legacy configurations docs#918
  • Remove HOSTNAME_FROM_LAMBDA from deprecations. This was deprecated but then added to the new Lambda provider upon request. The Lambda team decided to un-deprecate it given the need and currently no supported alternatives.
  • Migrate a leftover AWS_ENDPOINT_URL
  • Remove some outdated comments with deprecated references to LOCALSTACK_HOSTNAME

@joe4dev joe4dev added the semver: patch Non-breaking changes which can be included in patch releases label Nov 15, 2023
@joe4dev joe4dev added this to the 3.0 milestone Nov 15, 2023
@joe4dev joe4dev self-assigned this Nov 15, 2023
@joe4dev joe4dev marked this pull request as ready for review November 15, 2023 20:10
@joe4dev joe4dev requested review from dominikschubert and removed request for dominikschubert November 15, 2023 20:11
@@ -47,8 +47,6 @@ awslocal kinesis list-streams
}
```

**UPDATE**: Use the environment variable `$LOCALSTACK_HOSTNAME` to determine the target host inside your Lambda function. See [Configuration](#configuration) section for more details.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HarshCasper Are these outdated READMEs under docs still relevant here. Maybe we should remove them and link to our documentation page instead?

Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, seems to make sense to include the legacy config here as well.

Thanks for continuing to support the effort of improving our config management. Really looking forward to give this more structure in the future 👍

# Note: do *not* include DATA_DIR in this list, as it is treated separately
# => Synchronize this list with the above and the configuration docs:
# https://docs.localstack.cloud/references/configuration/
# => Sort this list alphabetically
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good hint! definitely a piece of knowledge that was mostly just lived through tradition but it's good to have this written down here

Copy link

LocalStack Community integration with Pro

       2 files         2 suites   1h 6m 57s ⏱️
2 323 tests 2 024 ✔️ 299 💤 0
2 324 runs  2 024 ✔️ 300 💤 0

Results for commit a667110.

@coveralls
Copy link

Coverage Status

coverage: 84.02% (+0.008%) from 84.012%
when pulling a667110 on organize-config-list
into 994350a on master.

@joe4dev joe4dev merged commit 567b51a into master Nov 15, 2023
29 checks passed
@joe4dev joe4dev deleted the organize-config-list branch November 15, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants