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

Remove localstack client dependency #9368

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Oct 17, 2023

Motivation

The localstack-client package used to define some of the core internals of LocalStack, particularly the service endpoints. This is a leftover from before we handled all requests through the single edge port 4566. Now that we don't use per-service ports, we have no need for the localstack-client package.

Changes

This PR has been open for a while, where previous changes removed the usage from within the Python code base itself. However, after rebasing recently I found no usages of the package in the code itself. The only change remaining is to stop installing the package. It is currently in the list of dependencies to support usage in -ext (arguably the dependency should have been moved there at some point, but it's not important), so we can remove it from this repo.

@simonrw simonrw added the semver: major Breaking changes which can be included in a major release only label Oct 17, 2023
@simonrw simonrw self-assigned this Oct 17, 2023
@simonrw simonrw force-pushed the remove-localstack-client-usage branch from 24a55be to 4be72bc Compare October 17, 2023 12:02
@github-actions
Copy link

github-actions bot commented Oct 17, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 6m 37s ⏱️
2 366 tests 2 051 ✔️ 315 💤 0
2 367 runs  2 051 ✔️ 316 💤 0

Results for commit 3dfe816.

♻️ This comment has been updated with latest results.

@simonrw simonrw added semver: patch Non-breaking changes which can be included in patch releases and removed semver: major Breaking changes which can be included in a major release only labels Nov 9, 2023
@simonrw simonrw force-pushed the remove-localstack-client-usage branch from 4be72bc to c7117eb Compare November 9, 2023 13:46
@simonrw simonrw added this to the 3.1 milestone Nov 13, 2023
@simonrw simonrw force-pushed the remove-localstack-client-usage branch from c7117eb to 3dfe816 Compare November 27, 2023 16:20
@simonrw simonrw changed the title Remove localstack client usage Remove localstack client import Nov 27, 2023
@simonrw simonrw changed the title Remove localstack client import Remove localstack client dependency Nov 27, 2023
@coveralls
Copy link

Coverage Status

coverage: 84.069% (-0.02%) from 84.089%
when pulling 3dfe816 on remove-localstack-client-usage
into 1e4aa80 on master.

@simonrw simonrw marked this pull request as ready for review November 27, 2023 18:42
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Awesome! Great to see that all the usages of localstack-client vanished over time, and the only thing left is the removal of the dependency! 🚀 🧹

@simonrw simonrw merged commit 3d51917 into master Nov 28, 2023
32 checks passed
@simonrw simonrw deleted the remove-localstack-client-usage branch November 28, 2023 15:52
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