Skip to content

Support customised service loader#1555

Draft
nicobrevin wants to merge 2 commits into
Unidata:maint-5.xfrom
nicobrevin:support-customised-service-loader
Draft

Support customised service loader#1555
nicobrevin wants to merge 2 commits into
Unidata:maint-5.xfrom
nicobrevin:support-customised-service-loader

Conversation

@nicobrevin
Copy link
Copy Markdown

@nicobrevin nicobrevin commented May 12, 2026

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

  • Link to any issues that the PR addresses
  • Add labels
  • Open as a draft PR
    until ready for review
  • Make sure GitHub tests pass
  • Mark PR as "Ready for Review"

@nicobrevin nicobrevin requested a review from lesserwhirls as a code owner May 12, 2026 10:10
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

This allows library consumers to control the classloader in applications
with non-trivial classloading requirements.
@nicobrevin nicobrevin force-pushed the support-customised-service-loader branch from 4981fc1 to 43361a8 Compare May 12, 2026 10:13
import thredds.filesystem.MFileOS7;
import ucar.nc2.internal.ncml.NcmlReader;
import ucar.nc2.util.NcServiceLoader;
import ucar.nc2.util.log.LoggerFactory;
Copy link
Copy Markdown
Member

@lesserwhirls lesserwhirls May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like switching out org.slf4j.LoggerFactory for ucar.nc2.util.log.LoggerFactory broke compilation.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@lesserwhirls
Copy link
Copy Markdown
Member

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 ./gradlew spotlessCheck, and to apply frixes you can run ./gradlew spotlessApply. Once the github tests pass, I'll run our full set of tests on our internal Jenkins just to be safe. At that point, we should be good to go and this would get in for the 5.10.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants