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

Only ignore profile activation and inclusion if the property source is profile specific #2613

Merged

Conversation

ryanjbaxter
Copy link
Contributor

If we set the flag on all property sources (like we do currently) then any profiles that should be activated via spring.profiles.active or spring.profiles.include coming from the config server will not be activated in the application.

@ryanjbaxter ryanjbaxter added this to the 4.1.4 milestone Oct 30, 2024
@spencergibb
Copy link
Member

In practice, how does this affect the client app?

@ryanjbaxter
Copy link
Contributor Author

What ends up happening is that if any of the property sources that come from the config server contains spring.profiles.active or spring.profiles.include none of those profiles will be activated in the client. So if you went to /acturator/env you wouldn't see any of those profiles active.

This could result in 2 bugs and I am sure there would be more....

  1. Configuration classes or beans that would only be active if a certain profiles is active would not be configured.
  2. If you had config data sources outside of the config server that should be activated with profiles being activated from the config server those would not be activated.

@spencergibb
Copy link
Member

So should this go only in main then for the behavior change, or does it fix bugs that should go in 4.1.x?

@ryanjbaxter
Copy link
Contributor Author

It would be a change in behavior but IMO its a change in behavior that is fixing a bug. Maybe we should talk with the rest of the team to see what they think?

@spencergibb
Copy link
Member

yup, we can chat tomorrow

@ryanjbaxter ryanjbaxter merged commit 92325c3 into spring-cloud:main Oct 31, 2024
1 check passed
ryanjbaxter pushed a commit that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants