Logger failes to initialize when not providing the Classic Logback Logger in 0.45.0 #1421
Comments
|
Got same exception while trying to update ktlint version for maven plugin. Here is stacktrace |
|
Same issue while using ktlint-gradle |
|
I'm seeing something similar with ktlint 0.45.0 running in Gradle with Spotless: |
|
@ZacSweers @z3d1k @charleskorn Did you upgrade from Ktlint 0.44.0 or from an older version? Im am unfamiliar with using Spotless, the gradle-plugin and the maven plugin myself as I always use the CLI only. So it might take a little longer, or I might need help, to reproduce and solve the problem. |
It might be accidently that this problem did not show up in ktlint 0.44.0 in your cases as the offending call then only happened in the special case that environment setting |
Yep, this was an upgrade from 0.44.0 for me. Examples: charleskorn/kaml#255 and batect/docker-client#129 |
|
@paul-dingemans yes, I've updated it from 0.44.0 In that version cast to logback was performed only if Just checked, if I would add |
|
Having the same error after upgrading to 0.45.0 |
Yes, I was hoping for this. I am working on a fix. |
|
Basically, I will just verify if a Logback Logger is provided before trying to adjust the loglevel. If a non Logback Logger is provided and the required loglevel is not set then a warning will be printed but processing continues without logging at that level. I will let you know when a new snapshot is published. I expect this snapshot to be available in about 24 hours. |
|
I believe that it would be better to move logback dependency from ktlint-core package to cli and introduce some kind of modifier option for the logger. |
Yeah, that would be fine by me as well. At least the idea sounds better than what I intended to do. I will post my PR anyways and have no problem with dropping it if your solution is better. |
|
@z3d1k I'd very much appreciate that approach instead. In my case I have an integration that is not depending on the CLI, but breaks just like the other reports in this ticket. |
2 tasks
paul-dingemans
added a commit
to paul-dingemans/ktlint
that referenced
this issue
Mar 20, 2022
…k logger For non-Logback loggers, it is only checked whether the enabled log level of the provided logger, is matching with the loglevel that Ktlint expects based on settings. Currently, only the Logback Classic logger (ch.qos.logback.classic.Logger) allows to change it loglevel dynamically. Closes pinterest#1421
5 tasks
@z3d1k There is an implicit requirement which makes me doubt whether your approach is going to work. The See #1425 for my solution. It at least works for me from the perspective as Ktlint developer. I was not able to verify whether this would also work from the perspective of an API user that is not providing a Logback logger. Any help on that would be appreciated so that we can add unit tests in Ktlint to prevent this error from happening again in the future. |
paul-dingemans
changed the title
paul-dingemans
changed the title
|
I missed that your #1424 was already posted before I dropped mine. Your solution does work for my case as well. It is looking great! I will drop some review remarks later today. Tnx for your contribution. |
paul-dingemans
added a commit
to z3d1k/ktlint
that referenced
this issue
Mar 20, 2022
…lic constant which were part of the public API to inform API users about its removal. Closes pinterest#1421
paul-dingemans
added a commit
that referenced
this issue
Mar 20, 2022
Closes #1421 Co-authored-by: paul-dingemans <paul-dingemans@users.noreply.github.com>
|
@z3d1k Thanks for your really nice solution for this problem. The change has been merged to master. I have requested a new release to be build. I regret that we published a release containing a blocking bug for our API users. I would like to take advantage of this situation however to connect more closely to you. In future releases we want to freshen up our API and it would be great if we can discuss and test those changes before releasing. I am however not sure what the best way would be to involve you. I see following possibilties:
Please let me know whether you would like to be involved by replying to this thread and what way of communication would suit you best. |
|
@paul-dingemans @z3d1k thanks a lot for fixing the issue. What about a new release rather than updating snapshot? |
I have requested a new release to be build as I can not publish it myself. It is expected to be released tomorrow by @shashachu |
I'm open to any of the proposed ways of involvement, slack probably will allow reaching out to wider community. |
|
I think the easiest as approach would be to do RCs when you have what may be a significant change, those are easy to test. Otherwise I think it would be wise to possibly set up integration tests against known popular consuming tools like spotless and just release quickly and more frequently. |
|
Hi all, 0.45.1 was just released. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Expected Behavior
It should work
Observed Behavior
This failure occurs (when using via Spotless)
Steps to Reproduce
Use editorconfig and ktlint 0.45.0 in a project
Your Environment
.editorconfigsettingsThe text was updated successfully, but these errors were encountered: