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

Fix LogbackLoggerConfigurator#init #8378

Merged
merged 1 commit into from Apr 27, 2018

Conversation

gmethvin
Copy link
Member

@gmethvin gmethvin commented Apr 23, 2018

I changed this in #8275 without realizing this was only used in initializing the dev mode server before an application is available, and is intentionally more minimal for that reason. This change should be minimal risk, since it reverts to the previous behavior in 2.6.12.

This also fixes another issue, which is that the global logging mode (used for logger.forMode) should be set in configure.

@@ -63,6 +66,7 @@ class LogbackLoggerConfigurator extends LoggerConfigurator {

val properties = LoggerConfigurator.generateProperties(env, configuration, optionalProperties)

play.api.Logger.setApplicationMode(env.mode)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually fixes a separate bug. The mode should be set in configure not init since init is only called by the dev mode server and not the application itself.

@gmethvin gmethvin force-pushed the logback-init branch 2 times, most recently from 9bfb8e6 to 9dc8168 Compare April 23, 2018 07:04
@gmethvin gmethvin changed the title Bring back old behavior in LogbackLoggerConfigurator.init() Bring back old behavior in LogbackLoggerConfigurator#init Apr 23, 2018
@gmethvin gmethvin changed the title Bring back old behavior in LogbackLoggerConfigurator#init Fix LogbackLoggerConfigurator#init Apr 23, 2018
Copy link
Member

@marcospereira marcospereira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

ps.: I wonder if is possible to have a test for this.

@gmethvin gmethvin merged commit bba5734 into playframework:master Apr 27, 2018
@gmethvin gmethvin deleted the logback-init branch April 27, 2018 18:32
@marcospereira marcospereira added this to the Play 2.6.14 milestone Apr 27, 2018
@marcospereira marcospereira added the status:ready-to-release PRs that are merged, backported, etc. label May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready-to-release PRs that are merged, backported, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants