-
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
Move logging to @ConfigMapping #42157
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Thanks! Let's not merge this too quickly.
I wonder if 3.14 is the right version to merge this, or it would be better to wait for 3.16 to get branched.
For now, I implemented a workaround to support mixed extensions.
Sure |
On a separate discussion, I wonder if we should introduce another config phase, BUILD_AND_RUN_TIME, which means values are read and used both on build and runtime. Probably only logging requires this, but maybe there are other use cases. |
There is also a conflict in this |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Let's try to get this in for 3.16.
I rebased to get a fresh CI run.
@radcortez can you just have a look at my question?
quarkus.log.file.path=quarkus.log |
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.
This was a mistake in the config file or something changed?
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.
I believe there is a mistake in the configuration file. The quarkus.log.file
is a group:
quarkus/core/runtime/src/main/java/io/quarkus/runtime/logging/LogConfig.java
Lines 51 to 52 in 3f8c3b3
@ConfigDocSection | |
public FileConfig file; |
Also, there is no property with the parent's name.
Sure. Thank you! |
This comment has been minimized.
This comment has been minimized.
I fixed an import conflict. Let's wait for CI and merge! |
Thanks! |
Status for workflow
|
@ConfigRoot
and@ConfigMapping
#42114It would help to come up with fixes for:
quarkus.test.profile.tags
#35693Noticeable changes:
@ConfigMapping
allows you to map the same configuration multiple times (the@ConfigRoot
class implementation only allowed one matching property). So,quarkus.console.color
is now available explicitly in both build time and runtime.LogRuntimeConfig
is also used during build time, but the@ConfigRoot
limitation required theConfigInstantiator
to create it. SinceLogBuildTimeConfig
,LogRuntimeConfig
, andConsoleRuntimeConfig
are required to set the build time and the runtime log, they are always registered by default, meaning they can be retrieved directly fromSmallRyeConfig
without any additional hacks@ConfigMapping
.