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

4.x: Amends datasource-ucp to depend on ucp11 instead of ucp #9353

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ljnelson
Copy link
Member

This PR changes integrations/cdi/datasource-ucp/pom.xml to depend on ucp11 instead of ucp, since the pom.xml was previously amended (via ef6649f) to include a dependency on our ojdbc integration rather than on the Oracle JDBC driver directly—a decision which had some unintended ripple effects.

Part of this change seems to have been to include the full production suite of artifacts related to the driver. Notably, the suite of artifacts also includes ucp11, along with several other jars related to observability and so on.

Our ojdbc integration excludes ucp from its dependency tree. But that dependency is never included (despite the Developers Guide For Oracle JDBC on Maven Central claiming (incorrectly) that it is (see "Use Case 1: I Want the Production Jars", second bullet point, and contrast with what the relevant pom.xml actually says)), so the exclusion is effectively a no-op. I'll fix this in a subsequent PR (though I'm not the original author, it seems simple enough).

Anyway, the net effect of all this, as #9349 indicates, is that a user of this project will currently end up with two copies of UCP on the classpath: one compiled with JDK 8 (ucp) dragged in by integrations/cdi/datasource-ucp, and another compiled with JDK 11 (ucp11) (dragged in by integrations/db/ojdbc). The intent of this PR is to fix this problem and remove at least some of this confusion.

Closes #9349.

@ljnelson ljnelson added bug Something isn't working cdi CDI jpa/jta integration 4.x Version 4.x labels Oct 10, 2024
@ljnelson ljnelson self-assigned this Oct 10, 2024
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Oct 10, 2024
@barchetta barchetta mentioned this pull request Oct 10, 2024
16 tasks
<groupId>io.smallrye</groupId>
<artifactId>jandex</artifactId>
<groupId>io.helidon.integrations.db</groupId>
<artifactId>ojdbc</artifactId>
Copy link
Member Author

Choose a reason for hiding this comment

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

The prior version had our ojdbc integration artifact brought in at compile time, which is not necessary (only ucp11 is necessary at compile time). That means, too, that perhaps this dependency is not needed? I should say: it definitely is not needed but I seem to recall others having strong opinions here, i.e. that pulling in the complete suite of Oracle production-level ojdbc-related artifacts at runtime is a convenient and proper thing to do. Personally I would vote to remove this dependency entirely and put the onus on the user to include it if she wants. What say others, particularly @arjav-desai and @barchetta?

Copy link
Member

@barchetta barchetta Oct 15, 2024

Choose a reason for hiding this comment

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

I found this in the Oracle JDBC Maven Guide: "As of DB 19c, UCP has a compile time dependency on Oracle JDBC; this is being fixed as we speak and will materialize in the next release (DB21c and up). "

So at one point maybe this was necessary? We might want to leave the dependency (as runtime) for backwards compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds reasonable.

@@ -44,10 +44,6 @@
<artifactId>ojdbc11-production</artifactId>
<type>pom</type>
<exclusions>
Copy link
Member Author

@ljnelson ljnelson Oct 16, 2024

Choose a reason for hiding this comment

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

Note: I deliberately removed a non-functional exclusion that was excluding the ucp artifact, which does not appear in the ojdbc11-production:pom artifact's dependencies (thus the exclusion of ucp had no effect). Since roughly February users have had ucp11 on their classpath, since it was not being excluded. This PR preserves ojdbc11-production:pom's set of dependencies, with the sole exception of xmlparserv2 (we continue to exclude that and include xmlparserv2_sans_jaxp_services in its place as before).

Copy link
Member Author

Choose a reason for hiding this comment

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

Per instructions I will be re-introducing an <exclusion>, but this time one that will exclude ucp* (most particularly ucp11). This means indirect consumers of this pom-typed artifact who might have been accustomed to having ucp11 on their classpath will now have to include ucp11 explicitly. We need to update the examples to make this easy to understand.

@ljnelson
Copy link
Member Author

Closing; incorporated in #9441

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x bug Something isn't working cdi CDI integration jpa/jta OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
Status: Sprint Scope
Development

Successfully merging this pull request may close these issues.

UCP integration needs to depend on ucp11, not ucp
2 participants