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

Fix Jena's initialization (#243) #244

Merged
merged 2 commits into from
Dec 15, 2023
Merged

Fix Jena's initialization (#243) #244

merged 2 commits into from
Dec 15, 2023

Conversation

jindrichmynarz
Copy link
Contributor

Summary

Jena's initialization is implemented using the ServiceLoader mechanism. This is respected when Neptune's JDBC driver repackages Jena.

However, when Jena is relocated under the shadow namespace shadow.org.apache.jena, this namespace is not reflected in the META-INF/services/shadow.org.apache.jena.sys.JenaSubsystemLifecycle provider-configuration file, where the original namespace (i.e. org.apache.jena) is used.

Moreover, when packaging the Neptune JDBC driver, Jena is minimized. Since the ServiceLoader providers (i.e. shadow.org.apache.jena.riot.system.InitRIOT, shadow.org.apache.jena.sparql.system.InitARQ, and shadow.org.apache.jena.sys.InitJenaCore) aren't referenced statically, they aren't included in the minimized package, which prevents Jena's dynamic initialization.

Description

Rewriting fully qualified class names in the provider-configuration file can be addressed by mergeServiceFiles() that uses ServiceFileTransformer.

Jena's dynamically loaded ServiceLoader providers can be included by excluding Jena from minimization, similarly to the other exclusions currently used.

Related Issue

#243

Additional Reviewers

@birschick-bq
@xiazcy
@alexey-temnikov
@kylepbit
@Cole-Greer

@Cole-Greer
Copy link
Contributor

Hi @jindrichmynarz, thanks for submitting this PR! Your changes and reasoning makes sense to me although I'm not sure what's happening with the tableau connector building in the CI. It appears unrelated to your changes and is currently failing in the develop branch as well. Out of an abundance of caution, I would like to investigate the tableau issue further before approving this PR. I will followup after investigating the issue.

Copy link
Contributor

@Cole-Greer Cole-Greer left a comment

Choose a reason for hiding this comment

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

Changes LGTM. Thanks for the submission!

@Cole-Greer Cole-Greer merged commit 07ef630 into aws:develop Dec 15, 2023
3 of 4 checks passed
@jindrichmynarz jindrichmynarz deleted the fix/jena-initialization branch January 3, 2024 20:02
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.

2 participants