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

CORE-3220 print detailed output when using logLevel INFO or greater #770

Closed
wants to merge 1 commit into from
Closed

CORE-3220 print detailed output when using logLevel INFO or greater #770

wants to merge 1 commit into from

Conversation

litpho
Copy link
Contributor

@litpho litpho commented May 16, 2018

In the current 3.6.x code, logLevel is not used to configure the root logger and the outputLogs field of the consoleLogFilter is never changed.

This change will keep the current behaviour if --logLevel is not used.
If --logLevel is used and it is INFO or greater, the root logger is set to this level and outputLogs will be set to true as well.

This will probably be of interest to people watching https://liquibase.jira.com/browse/CORE-3220 as well.

@stevesaliman
Copy link
Contributor

Out of curiosity, why limit the log level change to INFO or greater? Why not set the level to whatever the user specified?

@litpho
Copy link
Contributor Author

litpho commented Jun 30, 2018

The root logger is, in fact, set to the level of whatever the user specified.

What may not have been clear is that the "INFO or greater" threshold is used to activate the separate "outputLogs" switch. That could have been an "Anything but OFF" switch as well, which may have been less confusing.

@stevesaliman
Copy link
Contributor

It looks to me like the Level.INFO.isGreaterOrEqual(newLevel) code will prevent debug level output from going to the console. I actually like the "anything but OFF" idea. I think all we'd need to do is remove the INFO comparison.

@prerakalw
Copy link

You can use https://github.com/prerakalw/liquibase-with-logs version as a temporary fix.

@jimzucker
Copy link

You can use https://github.com/prerakalw/liquibase-with-logs version as a temporary fix.

@prerakalw Is there a GitHub branch with this changes that we can review and build ourselves?

@hajdukd
Copy link

hajdukd commented Mar 13, 2019

Why it takes so long to merge such a crucial fix ?

@huytmb
Copy link

huytmb commented Apr 14, 2019

This issue has not been resolved :(

@prerakalw
Copy link

@huytmb try the attached jar.
@jimzucker did you try the attached jar? is it solving your problem..

@asdfklgash
Copy link

@huytmb try the attached jar.
@jimzucker did you try the attached jar? is it solving your problem..

For me it didn't work.

@huytmb
Copy link

huytmb commented Apr 14, 2019

@huytmb try the attached jar.
@jimzucker did you try the attached jar? is it solving your problem..

Thank @prerakalw, i'll try it.
At the moment, I decided to downgrade to use liquibase 3.5.5.

@jhartotr
Copy link

Will liquibase 3.5.5 work with spring boot 2.1.5?

@darioseidl
Copy link

darioseidl commented Jun 16, 2019

For anyone using Gradle, the liquibase-gradle-plugin also has a workaround for this: OutputEnablingLiquibaseRunner (see their readme).

Is there anything stopping this pull request from being merged? It looks like a very simple fix and a lot of people are waiting for this. The only thing it doesn't address is that the SQL statements are also logged on info level now, which may be excessive, but that is less of a concern and could be addressed separately. Just having a way to enable logging at all would be important.

@datical-jenkins datical-jenkins changed the title CORE-3220 print detailed output when using logLevel INFO or greater LB-117 ⁃ CORE-3220 print detailed output when using logLevel INFO or greater Mar 4, 2020
@datical-jenkins datical-jenkins changed the title LB-117 ⁃ CORE-3220 print detailed output when using logLevel INFO or greater CORE-3220 print detailed output when using logLevel INFO or greater Mar 5, 2020
@ro-rah
Copy link

ro-rah commented Jun 17, 2020

Hi @litpho , could I move this PR to be merged into our upcoming 3.10.x release? Or does it have to be on the 3.6.x? The 3.10.x would be the faster way to go.

Sorry for the long delay. Up until a couple of months ago, @nvoxland was the only one working on the community side but, we have a community team tasked with getting PRs evaluated. Thanks for the patience.

@stevesaliman
Copy link
Contributor

I'm not sure this PR is needed in 3.10.x. The way logging is initialized seems to be one of the things that tends to change from version to version, and it looks like it works properly in the 3.10.x branch.

At least when I tested it with the Liquibase Gradle plugin, the logLevel switch had the desired effect.

@ro-rah
Copy link

ro-rah commented Jun 17, 2020

@stevesaliman ! Thanks for weighing in, looks like it is then a version specific fix. @litpho do you still needed this PR merged to the 3.6.x for a fix to get made? Just for full disclosure, we are prioritizing latest release (3.10.x at this writing) and master merges (for 4.0 Beta).

@ro-rah ro-rah changed the base branch from 3.6.x to 3.10.x August 24, 2020 20:32
@molivasdat
Copy link
Contributor

Closing as not going to merge since already fixed in 3.10.x release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet