firegloves / MemPOI

A library to simplify export from database to Excel files using Apache POI :japanese_goblin:
MIT License
57 stars 7 forks source link

Cast to LogBack-Logger hinders use in other libraries with SLF4J bindings like Maven #6

Closed zaplatynski closed 3 years ago

zaplatynski commented 4 years ago

https://github.com/firegloves/MemPOI/blob/1ce750e149bed930ce426898d007493539f1a34f/src/main/java/it/firegloves/mempoi/config/MempoiConfig.java#L37

I want to create a Maven plugin to export database tables to excel but I got during runtime follwoing exception:

org.slf4j.impl.MavenSimpleLogger cannot be cast to ch.qos.logback.classic.Logger

Please don't use concrete logger classes while using SLF4J. That brakes the contract or intend of SLF4J.

Thanks in advance!

P.S. You should use system properties to influence the log level: http://logback.qos.ch/manual/configuration.html

firegloves commented 4 years ago

Hello Marian,

thanks for your advice. I'm already working to a new version where logback will be evicted from MemPOI, it will use plain java logging API. Unfortunately I'm working hard during these days and I'm a bit slowed down.

Thanks again for your help

See you soon

--

Luca Corsetti

Il giorno lun 6 lug 2020 alle ore 17:15 Marian Zaplatynski < notifications@github.com> ha scritto:

https://github.com/firegloves/MemPOI/blob/1ce750e149bed930ce426898d007493539f1a34f/src/main/java/it/firegloves/mempoi/config/MempoiConfig.java#L37

I want to create a Maven plugin to export database tables to excel but I got during runtime follwoing exception:

org.slf4j.impl.MavenSimpleLogger cannot be cast to ch.qos.logback.classic.Logger

Please don't use concrete logger classes while using SLF4J. That brakes the contract or intend of SLF4J.

Thanks in advance!

P.S. You should use system properties to influence the log level: http://logback.qos.ch/manual/configuration.html

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/firegloves/MemPOI/issues/6, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHOPDNJT4VZS72UG4ZEK3DR2HTBJANCNFSM4ORWUATA .

firegloves commented 4 years ago

Hi Marian,

thinking back at your email, I was pondering about what you said. I never developed a third-party library so I decided to make a small research on the web in order to understand better.

I'm not really expert supplying log functionalities and I was choosing JUL aiming to be more flexible as possible, but now I think to go ahead with SLF4J following your suggestion.

Because I'm renewing the log system right now, it could be a good time to make some decisions. If for any reason you would like to collaborate or share something with me, I would be grateful to you.

Thanks again Bye

--

Luca Corsetti

Il giorno lun 6 lug 2020 alle ore 18:01 Luca Corsetti lucorset@gmail.com ha scritto:

Hello Marian,

thanks for your advice. I'm already working to a new version where logback will be evicted from MemPOI, it will use plain java logging API. Unfortunately I'm working hard during these days and I'm a bit slowed down.

Thanks again for your help

See you soon

--

Luca Corsetti

Il giorno lun 6 lug 2020 alle ore 17:15 Marian Zaplatynski < notifications@github.com> ha scritto:

https://github.com/firegloves/MemPOI/blob/1ce750e149bed930ce426898d007493539f1a34f/src/main/java/it/firegloves/mempoi/config/MempoiConfig.java#L37

I want to create a Maven plugin to export database tables to excel but I got during runtime follwoing exception:

org.slf4j.impl.MavenSimpleLogger cannot be cast to ch.qos.logback.classic.Logger

Please don't use concrete logger classes while using SLF4J. That brakes the contract or intend of SLF4J.

Thanks in advance!

P.S. You should use system properties to influence the log level: http://logback.qos.ch/manual/configuration.html

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/firegloves/MemPOI/issues/6, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHOPDNJT4VZS72UG4ZEK3DR2HTBJANCNFSM4ORWUATA .

zaplatynski commented 4 years ago

Well, no worries, everyone had a start once. Your work is great here and I was happy to find a way to save a SQL statement direct to an Excel file.

From my side it is fine to work against SLF4J API since it let you choose the logging implementation, be it Logback, Log4J or even JUL or some own implementation like Maven. To understand how SLF4J works, have a look at: http://www.slf4j.org/legacy.html It describes how you could integrate in every logging situation.

For things like a debug mode it is fine to work with system properties or even OS environment variables. But you should not force a cast to a specific logging class since that makes it hard to integrate your liberary somewhere else. Logging frameworks like Logback or Log4J supports the setting of system properties too.

I could try a pull request if you like.

firegloves commented 4 years ago

Thank you very much, it's a pleasure to help other developers to save their time :)

Yes, these argumentations are these ones that convinced me to proceed with SLF4J. Thank you for the references, I'll dig into this subject.

What I had in mind is exactly what you described and you are not the first user asking for this change. Sure, if you want to contribute with a pull request it would be great, just please target dev branch in the PR.

Thanks in advance

zaplatynski commented 3 years ago

I just put a pull request for the changes I sugguested