Skip to content

Conversation

@bentsku
Copy link
Contributor

@bentsku bentsku commented Oct 27, 2023

Motivation

Disclaimer: targeting master for now to validate all tests, and we will set the STRICT_SERVICE_LOADING variable to default to True in the release branch afterwards. This variable won't be advertised and is used to decouple eager service loading from "strict" service loading, and might be removed after the release.

When some services are problematic, or we want to use LocalStack for only one or more services, we should be able to use the SERVICES variable coupled with STRICT_SERVICE_LOADING=1 in order to be able to only enable the listed services.

The EAGER_SERVICE_LOADING functionality is untouched for now, and is applied on top: meaning if you don't have STRICT_SERVICE_LOADING set then you'll have all available services, and will only eager load the services in SERVICES.

If STRICT_SERVICE_LOADING=1 is set, then the services in SERVICES will be the only ones available. You can additionally eager load them by specifying EAGER_SERVICE_LOADING =1.

Example: you only want S3 and SQS loaded:
SERVICES=sqs,s3 + STRICT_SERVICE_LOADING=1

You want kinesis eager loaded at startup, but still lazy load other services:
SERVICES=kinesis + EAGER_SERVICE_LOADING =1

You want to only use kinesis, dynamodb, s3 and sqs, but with eager loading:
SERVICES=kinesis,dynamodb,sqs,s3 + STRICT_SERVICE_LOADING=1 + EAGER_SERVICE_LOADING =1

We should also do a new pass on the API_DEPENDENCIES constant to validate the inter-service dependencies, and be sure we will load the necessary services.

\cc @steffyP

Changes

Change the logic for get_enabled_apis, which is now linked to STRICT_SERVICE_LOADING.
Create new logic for eager loading, linked to the get_preloaded_services method and EAGER_SERVICE_LOADING .
Add a test to validate the new behaviour of disabling services.

@bentsku bentsku added area: asf semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases labels Oct 27, 2023
@bentsku bentsku self-assigned this Oct 27, 2023
@coveralls
Copy link

coveralls commented Oct 27, 2023

Coverage Status

coverage: 82.543% (-0.01%) from 82.555% when pulling 4504510 on services-config into b303510 on master.

@github-actions
Copy link

github-actions bot commented Oct 27, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 8m 56s ⏱️
2 327 tests 1 750 ✔️ 577 💤 0
2 328 runs  1 750 ✔️ 578 💤 0

Results for commit 4504510.

♻️ This comment has been updated with latest results.

@bentsku bentsku force-pushed the services-config branch 2 times, most recently from 9f9daab to df388ef Compare October 30, 2023 15:40
@bentsku bentsku requested a review from alexrashed October 31, 2023 12:31
@bentsku bentsku marked this pull request as ready for review October 31, 2023 12:32
@bentsku bentsku requested a review from thrau as a code owner October 31, 2023 12:32
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.

Nice! Really cool implementation of the strict service loading. Awesome that it stays pretty simple and clean. I only had a few comments, most of them are nitpicks :)

@bentsku bentsku requested a review from alexrashed October 31, 2023 16:07
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.

Sorry about the confusion with my (mistaken) PR comment in the last review.
Besides the new change concerning the port mapping, the code is looking great! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: asf semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants