Skip to content

re-introduce eager service loading #12657

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented May 23, 2025

Motivation

It seems we deleted eager service loading behavior with #11202 by accident.

Now with the new runtime framework, we can simply eager load services via hooks.

I've also added a proper bootstrap test to avoid deleting the behavior again without realizing it.

I updated the eager loading logic to not eager load all services if set, as it will most likely just wait forever.

My biggest question is where this hook declaration should live?

\cc @thrau @alexrashed

Changes

  • re-add eager loading via hooks
  • add bootstrap tests
  • add eager loading logic to not eager load all possible services

@bentsku bentsku added this to the 4.5 milestone May 23, 2025
@bentsku bentsku added area: infrastructure Installation and startup of LocalStack and components semver: patch Non-breaking changes which can be included in patch releases labels May 23, 2025
@bentsku bentsku self-assigned this May 23, 2025
Copy link

github-actions bot commented May 23, 2025

Test Results - Preflight, Unit

21 601 tests  ±0   19 955 ✅ ±0   6m 13s ⏱️ +5s
     1 suites ±0    1 646 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 551b4ab. ± Comparison against base commit 893018f.

This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.
tests.unit.utils.test_bootstrap.TestGetPreloadedServices ‑ test_returns_default_service_ports
tests.unit.utils.test_bootstrap.TestGetPreloadedServices ‑ test_empty_services_returns_no_services

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented May 23, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 7s ⏱️ -2s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 551b4ab. ± Comparison against base commit 893018f.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented May 23, 2025

Test Results - Alternative Providers

597 tests   419 ✅  14m 55s ⏱️
  4 suites  178 💤
  4 files      0 ❌

Results for commit 551b4ab.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented May 23, 2025

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 42m 44s ⏱️ -3s
4 464 tests ±0  4 075 ✅ ±0  389 💤 ±0  0 ❌ ±0 
4 466 runs  ±0  4 075 ✅ ±0  391 💤 ±0  0 ❌ ±0 

Results for commit 551b4ab. ± Comparison against base commit 893018f.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented May 23, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 21m 20s ⏱️
4 821 tests 4 279 ✅ 542 💤 0 ❌
4 827 runs  4 279 ✅ 548 💤 0 ❌

Results for commit 551b4ab.

♻️ This comment has been updated with latest results.

@bentsku bentsku requested a review from dfangl May 24, 2025 01:23
@bentsku bentsku marked this pull request as ready for review May 24, 2025 01:23
@bentsku bentsku requested a review from simonrw as a code owner May 24, 2025 01:23
Copy link
Member

@pinzon pinzon left a comment

Choose a reason for hiding this comment

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

Interesting how we lost a feature and no one noticed 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: infrastructure Installation and startup of LocalStack and components 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.

2 participants