egeria icon indicating copy to clipboard operation
egeria copied to clipboard

Cassandra metadata extractor connector move to a separate repository?

Open cmgrote opened this issue 3 years ago • 16 comments

Should this connector be part of the core repository?

As I investigate #1087, this connector is revealing a dependency on the datastax driver, which itself places an upper-bound on the reactive elements that would otherwise be included for a migration to WebClient (from RestTemplate). The datastax driver places an upper bound on org.reactivestreams::reactive-streams at version 1.0.2, while the latter releases of io.projectreactor::reactor-core (3.3.x and now 3.4.x) depend on version 1.0.3 -- and the Spring release we're using (5.3.x) depends on version 3.4.x of reactor-core, placing a dependency on version 1.0.3 of reactive-streams...

I vaguely recall discussing that such a connector may better be placed in its own repository (like other connectors) -- was that captured already somewhere, or should we do that through this issue? (Or am I mis-remembering such a discussion?)

cmgrote avatar Nov 12 '20 17:11 cmgrote

Here's the build issue:

[INFO] --- maven-enforcer-plugin:3.0.0-M3:enforce (enforce-versions) @ cassandra-metadata-extractor-connector ---
[WARNING] Rule 5: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for org.reactivestreams:reactive-streams:1.0.2 paths to dependency are:
+-org.odpi.egeria:cassandra-metadata-extractor-connector:2.5-SNAPSHOT
  +-com.datastax.oss:java-driver-core:4.9.0
    +-org.reactivestreams:reactive-streams:1.0.2
and
+-org.odpi.egeria:cassandra-metadata-extractor-connector:2.5-SNAPSHOT
  +-org.odpi.egeria:data-platform-client:2.5-SNAPSHOT
    +-org.odpi.egeria:ffdc-services:2.5-SNAPSHOT
      +-org.odpi.egeria:spring-rest-client-connector:2.5-SNAPSHOT
        +-io.projectreactor:reactor-core:3.4.0
          +-org.reactivestreams:reactive-streams:1.0.3

cmgrote avatar Nov 12 '20 17:11 cmgrote

It's interesting that this appears to result in only a warning - I would have expected it to be worse.

I think that we definitely need to do more to decouple such build dependencies, whether or not we choose to move connectors (or other elements of ecosystem) to other repositories. My current understanding is that we are moving them to other repositories.

grahamwallis avatar Nov 13 '20 09:11 grahamwallis

Agreed - these are two distinct issues. Firstly ensuring the code can be decoupled at runtime so we remove these hard dependencies, and avoid bringing unnecessary code into the runtime (as well as allow third party code such as connectors to be added), as well as making the development/test/distributino/focus process earlier by moving to a seperate repo.

Cassandra is something I would move to an external repo - though which one.... perhaps worth a team discussion on the focus group call? Also note we need to get the dynamic loading sorted first

We should also bear in mind that although we would no longer get an error if moved to a seperate repo, the issue still exists as there could be multiple versions of the library in the classpath -- this is why within egeria we do that enforcement. Without any other changes I am unsure which version would be picked up.

Shading(ie renaming classes) is the traditional way to avoid this, but we probably need to look at this in more depth. I believe we have a slot at the next monthly meeting to discuss this?

Graham - I think we fail on warnings in this step, as per your expectation

planetf1 avatar Nov 13 '20 09:11 planetf1

Also to add, we have a gradle build in parallel where the dependency resolution is slightly different -- gradle's default behaviour is better (it always aims for the latest, compatible versions) though we currently have less additional checks.

Aso worth adding that even within a repo we can override the repo-wide policy from the top level pom, so moving to a new repo isn't a prereq (despite the fact I think it's sensible in this case.. for other reasons).. but the check makes us stop and think about what we really want, and what the behaviour should be.

planetf1 avatar Nov 13 '20 09:11 planetf1

Who owns this connector now? @mstrelchuk ?

planetf1 avatar Dec 08 '20 16:12 planetf1

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Feb 07 '21 00:02 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar May 01 '21 00:05 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Aug 08 '21 00:08 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Oct 16 '21 00:10 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Dec 17 '21 00:12 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Feb 16 '22 00:02 github-actions[bot]

It appears as if the cassandra metadata extractor is no longer in the source tree.

However there are still a few remaining references to cleanup All are minor/low impact...

  • a redundant entry remains in the top level pom.xml for the dependencyManagement, which should be removed
  • THIRD_PARTY.md contains the license info
  • We could remove the commented content in DataStoreConnectorsArchiveBuilder.java for clarity, along with the related build.gradle script in open-connector-archives

I notice we still have a reference to disabling the metrics for spring+cassandra, which I think could go, unless it's needed when janusgraph is configured to use cassandra in the graph repo. To be checked?

planetf1 avatar Feb 23 '22 17:02 planetf1

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Apr 25 '22 00:04 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jun 26 '22 00:06 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Aug 27 '22 00:08 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Oct 29 '22 00:10 github-actions[bot]