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

Make DynamicPropertyRegistrarBeanInitializer public #33593

Closed
wilkinsona opened this issue Sep 25, 2024 · 5 comments
Closed

Make DynamicPropertyRegistrarBeanInitializer public #33593

wilkinsona opened this issue Sep 25, 2024 · 5 comments
Assignees
Labels
in: test Issues in the test module type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

wilkinsona commented Sep 25, 2024

Overview

Affects: 6.2

Following up on this comment, we have prototyped a change to Spring Boot that is a step towards replacing its support for injecting a DynamicPropertyRegistry with support for DynamicPropertyRegistrar beans instead.

As explained in the comment in #33501, additional support for DynamicPropertyRegistrar is needed for dynamic property registration when using Testcontainers at development time. In tests, it isn't needed as the test context framework already enables the support through its context customizer.

The prototype of the changes to Boot currently uses reflection when defining a DynamicPropertyRegistrarBeanInitializer bean as the initializer is package-private. This has helped to verify the approach but isn't something that we'd want to ship.

Can Framework 6.2 please provide an SPI for enabling DynamicPropertyRegistrar support outside of the test context framework, or perhaps just make DynamicPropertyRegistrarBeanInitializer public so that we don't need to use reflection?

Related Issues

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 25, 2024
@sbrannen sbrannen added the in: test Issues in the test module label Sep 25, 2024
@sbrannen sbrannen self-assigned this Sep 25, 2024
@sbrannen sbrannen added this to the 6.2.0-RC2 milestone Sep 25, 2024
@sbrannen sbrannen added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 25, 2024
@sbrannen
Copy link
Member

Thanks for raising the issue, @wilkinsona.

Following up on this comment,

I apologize: I somehow missed that comment.

we have prototyped a change to Spring Boot that is a step towards replacing its support for injecting a DynamicPropertyRegistry with support for DynamicPropertyRegistrar beans instead.

That would of course be great if Spring Boot can benefit from the new DynamicPropertyRegistrar support.

Can Framework 6.2 please provide an SPI for enabling DynamicPropertyRegistrar support outside of the test context framework, or perhaps just make DynamicPropertyRegistrarBeanInitializer public so that we don't need to use reflection?

In Spring Framework, we don't have any support for "dynamic properties" other than in the spring-test module. In other words, dynamic properties are very test-centric from a Framework perspective. In light of that, we don't have any plans to enable DynamicPropertyRegistrar support outside of the test context framework.

However, I think it should be fine to make DynamicPropertyRegistrarBeanInitializer public.

Shall I go ahead and make that change this week so that you can try it out with 6.2 snapshots?

@wilkinsona
Copy link
Member Author

Yes please, Sam. Support that's only available in spring-test would work well for us as folks using Testcontainers at development time will already have a dependency on spring-test.

@sbrannen sbrannen changed the title Provide an SPI for enabling DynamicPropertyRegistar support outside of the test context framework Make DynamicPropertyRegistrarBeanInitializer public Sep 26, 2024
@sbrannen
Copy link
Member

DynamicPropertyRegistrarBeanInitializer is now public on main.

Feel free to try it out in upcoming 6.2 snapshots.

@wilkinsona
Copy link
Member Author

I've updated the prototype and it looks good to me. Thanks!

@sbrannen
Copy link
Member

Great! Glad to hear that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants