-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Support multiple profiles #28091
Support multiple profiles #28091
Conversation
/cc @ia3andy |
8bba86d
to
634476e
Compare
This comment has been minimized.
This comment has been minimized.
634476e
to
db01fb5
Compare
This comment has been minimized.
This comment has been minimized.
What's the status of this? Seems important to have :) |
I am waiting for the CI to pass to give it a try |
CI has passed 😉 |
I've tried it with different cases and it seems to work fine (and as I expect it to work). |
Approve? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I am not sure why this is called "initial work..." instead of "support multiple profiles"
As this PR is side-effect friendly, I think it would be wise to have another review. @gastaldi maybe? |
I'm going to rebase this on |
I think we should get this in |
This comment has been minimized.
This comment has been minimized.
@gsmet up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments
core/runtime/src/main/java/io/quarkus/runtime/configuration/ProfileManager.java
Show resolved
Hide resolved
core/runtime/src/main/java/io/quarkus/runtime/configuration/QuarkusConfigFactory.java
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Failing Jobs - Building 5cf686e
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 18 #- Failing: integration-tests/reactive-messaging-hibernate-orm
📦 integration-tests/reactive-messaging-hibernate-orm✖
|
/ping @gsmet |
@radcortez I think with two approvals we can merge? |
This being a big change I think making it Quarkus 3.0 might be a good idea no? |
Sure, just wanted to make sure that @gsmet was ok with this.
Hum, I thought you needed this sooner. I mean, it is a considerable change, but not so big that it requires a major version. |
@radcortez, I think @gsmet had plenty of time to give an opinion on this :) I am not sure if the CI is considered as passed or not, but if it is, you can merge |
The CI failure is unrelated. A timeout issue. Well, I'll be merging this then. |
Initial work to support #27760.
Get rid of calls to
ProfileManager#getActiveProfile
and rely on the profiles list coming from SmallRye Config.