[#18] Provide slf4j 2 compatibility - implement service loader
Fixes #18
- Implemented service loader
- All tests pass
TODO (need help):
- Check service loader implementation
- Restore the older logging bridges - jul and commons logging
@greglook : I've fixed the check for slf4j version but I ran into another one. I need your help figuring this one out.
Found uncommented reference to 'requiring-resolve' in logger classes:
src/java/dialog/logger/DialogServiceProvider.java: IFn resolve = RT.var("clojure.core", "requiring-resolve");
So this check is failing because I put the Service Provider implementation in dialog/ dir.
I took the code from the previous StaticLoggerBinder implementation which was in a different path.
The other path was not searched by test/check-dynamic-resolves
readonly MATCHES=$(grep -RE '^ *IFn.*"requiring-resolve"' src/java/dialog)
Why is the check here anyway? I don't know what solution to adopt in fixing this.
I chose to move the service provider class to package dialog.impl and update the check to look into dialog/logger
The purpose of that test is to make sure that Dialog doesn't ship with a significant performance penalty that is caused by a development-time affordance. In the normal case in the current implementation, the StaticLoggerBinder is responsible for loading the dialog namespaces and resolving the various function vars which the Java code needs to hook into Clojure:
https://github.com/amperity/dialog/blob/3933260fec9c88068cf091aa47d6fd8999194499/src/java/org/slf4j/impl/StaticLoggerBinder.java#L53-L63
This should only happen once in a real deployment with dialog, because locking the namespace loading monitor and re-resolving the var on every single logging call would be extremely expensive and slow. Those functions are then passed to the DialogFactory and then into each DialogLogger:
https://github.com/amperity/dialog/blob/3933260fec9c88068cf091aa47d6fd8999194499/src/java/org/slf4j/impl/StaticLoggerBinder.java#L69-L70
https://github.com/amperity/dialog/blob/3933260fec9c88068cf091aa47d6fd8999194499/src/java/dialog/logger/DialogFactory.java#L57-L58
However, as you can see, loggers are cached, so when you're actually developing Dialog itself, this can be really annoying as it causes you to need to restart the REPL process to pick up changes, even to Clojure code. So there are some commented-out blocks like this, which you can enable to make hot-reloading work again, while you're working on Dialog.
https://github.com/amperity/dialog/blob/3933260fec9c88068cf091aa47d6fd8999194499/src/java/dialog/logger/DialogLogger.java#L130-L133
The test script is ensuring that these blocks are never left uncommented on accident.
hi @greglook . we are using apache jena in our project and it's bound to slf4j-api 2.0. I'm going to look into using dialog with the API for our project (was on todo for some time).
Any chance we can get this PR merged in dialog? More and more projects will most likely move to slf4j-api 2.0 since there are really no downsides. And the API is backward compatible. Just need to make sure any implementations for < 2.0 are on the classpath.
Codecov Report
Base: 94.62% // Head: 94.62% // No change to project coverage :thumbsup:
Coverage data is based on head (
d656952) compared to base (060a5f9). Patch has no changes to coverable lines.
Additional details and impacted files
@@ Coverage Diff @@
## main #19 +/- ##
=======================================
Coverage 94.62% 94.62%
=======================================
Files 8 8
Lines 614 614
Branches 13 13
=======================================
Hits 581 581
Misses 20 20
Partials 13 13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.