Support customised service loader#1555
Conversation
|
|
nicobrevin
force-pushed
the
support-customised-service-loader
branch
from
4981fc1 to
43361a8
Compare
| import thredds.filesystem.MFileOS7; | ||
| import ucar.nc2.internal.ncml.NcmlReader; | ||
| import ucar.nc2.util.NcServiceLoader; | ||
| import ucar.nc2.util.log.LoggerFactory; |
There was a problem hiding this comment.
It looks like switching out org.slf4j.LoggerFactory for ucar.nc2.util.log.LoggerFactory broke compilation.
There was a problem hiding this comment.
Whoops, fixed. Also ran spotless, but all that did was remove some whitespace from my javadoc 🤷
If you think the change needs it, I can add tests, but I gave up quickly when I got errors about missing JDKs.
There was a problem hiding this comment.
I would not worry about tests for this change, as our existing tests should cover these codepaths quite well. Spotless can be picky about things such as whitespace, which gets annoying. Thanks again for your contribution!
|
I think this approach will work nicely. Thank you so much for your contribution, @nicobrevin! Besides the compilation issue (see above) and the CLA, the only other thing that's tripping up the GitHub tests is the code style. To check the code style, you can run |
This allows library consumers to control the classloader in applications with non-trivial classloading requirements.
No worries at all, @lesserwhirls , I appreciate your timely response and pragmatism :) |
nicobrevin
force-pushed
the
support-customised-service-loader
branch
from
43361a8 to
a37efdb
Compare
|
Just a quick follow up that our internal tests also passed! |
3 participants
Description of Changes
As per #1382, adds a custom ServiceLoader#load method that supports customizing the classloader. In our case we have something of a plugin architecture that isolates the different plugins in their own class loader. This causes chaos and hilarity when the netcdf plugin is used, as it is not going to execute that code with the netcdf library's classloader set as the calling thread's context classloader. I have worked around it to some extent by finaggling our code to do a best-effort and forcibly load some of the offending netcdf classes with the context class loader set correctly, but that only works for the code that loads services in a static initializer.
PR Checklist
until ready for review