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

Use SLF4J logging system for the library #15

Open
magicDGS opened this issue Jul 19, 2018 · 4 comments
Open

Use SLF4J logging system for the library #15

magicDGS opened this issue Jul 19, 2018 · 4 comments

Comments

@magicDGS
Copy link
Member

Instead of porting the Log class from HTSJDK I propose to use SLF4J instead to let the API user to choose the logging framework that they want.

The configuration will be to add slf4j as a compile dependency and some implementation for testing (e.g., org.slf4j:slf4j-simple). Promatically setting verbosity should be done by downstream application or passing a org.slf4j.Logger that can be configure by the caller. This can also allow to switch off the verbosity for the library and output only the logs for the downstream application.

Any objections @samtools/htsjdk-next-maintainers?

@lbergelson
Copy link
Member

This seems like a good idea to me in theory. It does add additional dependencies, but ultimately almost every downstream project is going end up with a logging framework in addition to the htsjdk Logger class.

As long as:

  1. We're able to package a default logger implementation so that people who import only htsjdk get log messages without having to do additional configuration.
  2. It's capable of being adjusted programmatically by downstream projects.
  3. The SLF4J dependency itself isn't super massive with a million of it's own dependencies.

Then I think it should be good.

@tfenne I feel like you're the most likely to object, what do you think?

@magicDGS
Copy link
Member Author

For your requirements, I suggest to include an module (htsjdk-logging or htsjdk-logging-utils), which will fix your concerns as following:

  1. The module can have custom implementation or a standard one (I recommend logback), so people importing the complete htsjdk artifact does not need to add additional stuff. An advance user can remove completely the dependency and add it's own
  2. The package can have methods to set the verbosity promatically. Assuming a concrete implementation bundled (e.g., logback), a method can implement the setLogLevel with its verbosity levels and getting the root logger. It is a pity that such methods does not exits in slf4j, but that was a design decision that won't change in the near future (I was always thinking to do a small project for each binding to add the functionality, but I've never got the chance).
  3. If I remember correctly, the API version 1.7.25 has no extra dependencies and the jar is 40KB. If we would like to have a very small dependency overload, the slf4j-simple is only 14KB. If we want to have a more flexible default, logback is the native implementation, which requires both logback-core and logback-classic dependencies, whose jars are 460KB and 283KB respectively. Other bindings are also available, so we can check sizes and number of other dependencies allowed.

@lbergelson
Copy link
Member

That sounds good to me. A few hundred kb doesn't seem like a problem to me.

@magicDGS
Copy link
Member Author

Cool - once we have the build system (#14) I will add the proposed module.

@magicDGS magicDGS self-assigned this Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants