Skip to content

partially revert to spring-core for properties emission#2741

Closed
metron2 wants to merge 2 commits intoPlaytikaOSS:developfrom
metron2:fix-dynamic
Closed

partially revert to spring-core for properties emission#2741
metron2 wants to merge 2 commits intoPlaytikaOSS:developfrom
metron2:fix-dynamic

Conversation

@metron2
Copy link
Contributor

@metron2 metron2 commented Jan 20, 2026

DynamicPropertyRegistrar seems be processed too late in many scenarios. This partial revert keeps the removal of spring-cloud but restores the use of MapPropertySource. This simplier approach ensures that the properties are emitted as soon as the container is started.

@Periecle Periecle self-assigned this Feb 6, 2026
@Periecle
Copy link
Collaborator

Periecle commented Feb 6, 2026

@metron2 thanks a lot for this contribution. Approved from my side, but I want to have second approval from @amizurov
P.S. Sorry for rather long review 😺

Periecle
Periecle previously approved these changes Feb 6, 2026
@Periecle Periecle enabled auto-merge (rebase) February 6, 2026 16:30
@metron2
Copy link
Contributor Author

metron2 commented Feb 6, 2026

I'll fix the test(s) that don't work if you like this approach

@Periecle
Copy link
Collaborator

Periecle commented Feb 6, 2026

@metron2 I think that your approach is correct, and probably the only way to leave containers as functional beans, while preserving proper configurations properties resolution. Please fix the tests

auto-merge was automatically disabled February 7, 2026 03:57

Head branch was pushed to by a user without write access

@codecov
Copy link

codecov bot commented Feb 7, 2026

Codecov Report

❌ Patch coverage is 85.38462% with 114 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.44%. Comparing base (3e4fd8b) to head (da13a13).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
...azurite/EmbeddedAzuriteBootstrapConfiguration.java 34.54% 36 Missing ⚠️
...ytika/testcontainer/toxiproxy/ToxiproxyHelper.java 0.00% 9 Missing ⚠️
...ory/EmbeddedArtifactoryBootstrapConfiguration.java 83.33% 3 Missing ⚠️
...andra/EmbeddedCassandraBootstrapConfiguration.java 78.57% 3 Missing ⚠️
...r/consul/EmbeddedConsulBootstrapConfiguration.java 72.72% 3 Missing ⚠️
...ntainer/db2/EmbeddedDb2BootstrapConfiguration.java 80.00% 3 Missing ⚠️
...h/EmbeddedElasticSearchBootstrapConfiguration.java 80.00% 3 Missing ⚠️
...ntainer/git/EmbeddedGitBootstrapConfiguration.java 80.00% 3 Missing ⚠️
...r/pubsub/EmbeddedPubsubBootstrapConfiguration.java 86.36% 3 Missing ⚠️
...storage/EmbeddedStorageBootstrapConfiguration.java 78.57% 3 Missing ⚠️
... and 15 more
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #2741      +/-   ##
=============================================
+ Coverage      81.82%   85.44%   +3.62%     
+ Complexity       852      625     -227     
=============================================
  Files            202      178      -24     
  Lines           2987     2845     -142     
  Branches         150      108      -42     
=============================================
- Hits            2444     2431      -13     
+ Misses           470      355     -115     
+ Partials          73       59      -14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@metron2
Copy link
Contributor Author

metron2 commented Feb 7, 2026

P.S. Sorry for rather long review 😺

No worries. Thanks for making such a handy library available.

@amizurov
Copy link
Contributor

amizurov commented Feb 8, 2026

@metron2 Thanks a lot, i think we should do a full revert of #2398 rather than a partial revert. Because it didn't actually remove spring-cloud from the project. Any good improvements from #2398 (Azurite ordering, log messages) can be cherry-picked in a separate small PR. @Periecle @ijusti WDYT ?

@ijusti
Copy link
Collaborator

ijusti commented Feb 9, 2026

reverted in #2770

@ijusti ijusti closed this Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants