search

scylladb / scylla-jmx

Scylla JMX proxy
GNU Affero General Public License v3.0
29 stars 53 forks source link

scylla-apiclient: drop hk2-locator dependency #234

Closed tchaikov closed 8 months ago

tchaikov commented 9 months ago

Drop the dependency of hk2-locator, in order to get rid of javaassist and other 3rd party dependencies of it.

there are two ways to address this problem:

  1. bump up the dependencies which depend on hk2-locator to a version which depend on hk2-locator 2.5.0. because hk2-locator 2.5.0 contains a change to drop the unnecessary dependencies which made their way into the BOM. but they should have not.
  2. bump up the dependencies which depend on hk2-locator to a version which does not depend on hk2-locator at all.

before this change, per the output of mvn dependency:tree -Dverbose=true, we indirectly depend on hk2-locator 2.4.0.

after this change, hk2-locator dependency is dropped by bumping up org.glassfish.jersey.core to the oldest stable version which was released (see https://mvnrepository.com/artifact/org.glassfish.jersey.core/jersey-common/2.27) after hk2-locator 2.5.0 was released (see https://mvnrepository.com/artifact/org.glassfish.hk2/hk2-locator/2.5.0-b42), otherwise we still depend on hk2-locator 2.4.0 indirectly.

verified by running

mvn dependency:tree -Dverbose=true | grep hk2-locator

nothing shows up with this change.

Fixes #231 Signed-off-by: Kefu Chai kefu.chai@scylladb.com

yaronkaikov commented 9 months ago 
bash-5.2$ mvn dependency:tree -Dverbose=true                   
[INFO] Scanning for projects...
[INFO] 
[INFO] --------------------< com.scylladb.jmx:scylla-jmx >---------------------
[INFO] Building Scylla JMX 1.0
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ scylla-jmx ---
[INFO] com.scylladb.jmx:scylla-jmx:jar:1.0
[INFO] +- com.scylladb.jmx:scylla-apiclient:jar:1.0:compile
[INFO] |  +- org.yaml:snakeyaml:jar:2.2:compile
[INFO] |  +- org.glassfish.jersey.core:jersey-common:jar:2.27:compile
[INFO] |  |  +- (javax.ws.rs:javax.ws.rs-api:jar:2.1:compile - omitted for conflict with 2.0.1)
[INFO] |  |  +- javax.annotation:javax.annotation-api:jar:1.2:compile
[INFO] |  |  +- org.glassfish.hk2.external:javax.inject:jar:2.5.0-b42:compile
[INFO] |  |  \- org.glassfish.hk2:osgi-resource-locator:jar:1.0.1:compile
[INFO] |  +- javax.ws.rs:javax.ws.rs-api:jar:2.0.1:compile
[INFO] |  +- javax.ws.rs:jsr311-api:jar:1.1.1:compile
[INFO] |  +- org.glassfish.jersey.core:jersey-client:jar:2.27:compile
[INFO] |  |  +- (javax.ws.rs:javax.ws.rs-api:jar:2.1:compile - omitted for conflict with 2.0.1)
[INFO] |  |  +- (org.glassfish.jersey.core:jersey-common:jar:2.27:compile - omitted for duplicate)
[INFO] |  |  \- (org.glassfish.hk2.external:javax.inject:jar:2.5.0-b42:compile - omitted for duplicate)
[INFO] |  +- org.glassfish:javax.json:jar:1.0.4:compile
[INFO] |  +- com.google.guava:guava:jar:32.1.3-jre:compile
[INFO] |  |  +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO] |  |  +- com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:compile
[INFO] |  |  +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO] |  |  +- org.checkerframework:checker-qual:jar:3.37.0:compile
[INFO] |  |  +- com.google.errorprone:error_prone_annotations:jar:2.21.1:compile
[INFO] |  |  \- com.google.j2objc:j2objc-annotations:jar:2.8:compile
[INFO] |  +- com.google.collections:google-collections:jar:1.0:compile
[INFO] |  +- javax.activation:activation:jar:1.1:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.15.3:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.15.3:compile
[INFO] |  |  +- (com.fasterxml.jackson.core:jackson-annotations:jar:2.15.3:compile - omitted for duplicate)
[INFO] |  |  \- com.fasterxml.jackson.core:jackson-core:jar:2.15.3:compile
[INFO] |  \- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.15.3:compile
[INFO] |     +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.15.3:compile
[INFO] |     |  +- (com.fasterxml.jackson.core:jackson-core:jar:2.15.3:compile - omitted for duplicate)
[INFO] |     |  \- (com.fasterxml.jackson.core:jackson-databind:jar:2.15.3:compile - omitted for duplicate)
[INFO] |     \- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.15.3:compile
[INFO] |        +- (com.fasterxml.jackson.core:jackson-annotations:jar:2.15.3:compile - omitted for duplicate)
[INFO] |        +- (com.fasterxml.jackson.core:jackson-core:jar:2.15.3:compile - omitted for duplicate)
[INFO] |        +- (com.fasterxml.jackson.core:jackson-databind:jar:2.15.3:compile - omitted for duplicate)
[INFO] |        +- jakarta.xml.bind:jakarta.xml.bind-api:jar:2.3.3:compile
[INFO] |        |  \- (jakarta.activation:jakarta.activation-api:jar:1.2.2:compile - omitted for duplicate)
[INFO] |        \- jakarta.activation:jakarta.activation-api:jar:1.2.2:compile
[INFO] \- junit:junit:jar:4.13.1:test
[INFO]    \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.750 s
[INFO] Finished at: 2024-01-08T09:17:08Z
[INFO] ------------------------------------------------------------------------

@mykaul

tchaikov commented 9 months ago

being tested at https://github.com/scylladb/scylladb/pull/16680

mykaul commented 9 months ago

+- (javax.ws.rs:javax.ws.rs-api:jar:2.1:compile - omitted for conflict with 2.0.1)

tchaikov commented 9 months ago

+- (javax.ws.rs:javax.ws.rs-api:jar:2.1:compile - omitted for conflict with 2.0.1)

v2:

tchaikov commented 9 months ago

@yaronkaikov @mykaul thank you for your reviews. could you take another look?

mykaul commented 9 months ago

The only way I can review this is see the output of mvn dependency:tree -Dverbose=true on this patch and see that we are now better than before. then we have to somehow run CI on this to ensure we did not break anything, but that's an extra step later on, I reckon.

mykaul commented 9 months ago

Also, see https://github.com/scylladb/java-driver/blob/085dd34ef46b1de5111629bdccfe4056c4355676/pom.xml#L74 - unsure, but it makes sense that we'll align...

tchaikov commented 9 months ago

The only way I can review this is see the output of mvn dependency:tree -Dverbose=true on this patch and see that we are now better than before. then we have to somehow run CI on this to ensure we did not break anything, but that's an extra step later on, I reckon.

it's running at https://github.com/scylladb/scylladb/pull/16680

tchaikov commented 9 months ago

Also, see https://github.com/scylladb/java-driver/blob/085dd34ef46b1de5111629bdccfe4056c4355676/pom.xml#L74 - unsure, but it makes sense that we'll align...

unless it's urgent, i'd leave it to some one who actually understands Java better than me.

tchaikov commented 9 months ago

nodetool is failing. 

>               raise NodetoolError(" ".join(nodetool), exit_status, stdout, stderr)
E               ccmlib.node.ToolError: Subprocess /jenkins/workspace/scylla-master/gating-dtest-release/scylla/.ccm/scylla-repository/16680/share/cassandra/bin/nodetool -h 127.0.7.3 -p 7199 -Dcom.sun.jndi.rmiURLParsing=legacy drain exited with non-zero status; exit status: 1; 
E               stdout: nodetool: InjectionManagerFactory not found.
E               See 'nodetool help' or 'nodetool help <command>'.

see https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/5616/testReport/junit/alternator_tests/TesterAlternator/Tests___dtest___test_drain_during_dynamo_load/

so i'd have to add something like

<dependency>
    <groupId>org.glassfish.jersey.inject</groupId>
    <artifactId>jersey-hk2</artifactId>
    <version>2.27</version>
</dependency>
mykaul commented 9 months ago

@tchaikov - indeed - https://stackoverflow.com/questions/44088493/jersey-stopped-working-with-injectionmanagerfactory-not-found - that's what I was worried about all along.

tchaikov commented 9 months ago

v3:

tchaikov commented 9 months ago

@mykaul hi Yaniv, the test at https://github.com/scylladb/scylladb/pull/16680 is green now. could you take another look?

tchaikov commented 9 months ago 
$ mvn dependency:tree -Dverbose=true        
[INFO] Scanning for projects...                                                                                        
[WARNING]                                                                                                              
[WARNING] Some problems were encountered while building the effective model for com.scylladb.jmx:scylla-jmx:jar:1.0
[WARNING] 'build.plugins.plugin.version' for org.apache.maven.plugins:maven-compiler-plugin is missing. @ line 61, column 21
[WARNING]                                                                                                              
[WARNING] It is highly recommended to fix these problems because they threaten the stability of your build.
[WARNING]                                                                                                              
[WARNING] For this reason, future Maven versions might no longer support building such malformed projects.
[WARNING]                                                                                                              
[INFO]                                                                                                                 
[INFO] --------------------< com.scylladb.jmx:scylla-jmx >---------------------                            
[INFO] Building Scylla JMX 1.0                                                                                         
[INFO]   from pom.xml                                                                                                  
[INFO] --------------------------------[ jar ]---------------------------------                        
[INFO]                                                                                                                 
[INFO] --- dependency:2.8:tree (default-cli) @ scylla-jmx ---             
[WARNING] Parameter 'localRepository' is deprecated core expression; Avoid use of ArtifactRepository type. If you need access to local repository, switch to '${repositorySystemSession}' expression and get LRM from it instead.
[INFO] com.scylladb.jmx:scylla-jmx:jar:1.0                                                                             
[INFO] +- com.scylladb.jmx:scylla-apiclient:jar:1.0:compile
[INFO] |  +- org.yaml:snakeyaml:jar:2.2:compile     
[INFO] |  +- org.glassfish.jersey.core:jersey-common:jar:2.27:compile          
[INFO] |  |  +- (javax.ws.rs:javax.ws.rs-api:jar:2.1:compile - omitted for duplicate)
[INFO] |  |  +- javax.annotation:javax.annotation-api:jar:1.2:compile          
[INFO] |  |  +- org.glassfish.hk2.external:javax.inject:jar:2.5.0-b42:compile
[INFO] |  |  \- org.glassfish.hk2:osgi-resource-locator:jar:1.0.1:compile
[INFO] |  +- org.glassfish.jersey.core:jersey-client:jar:2.27:compile          
[INFO] |  |  +- (javax.ws.rs:javax.ws.rs-api:jar:2.1:compile - omitted for duplicate)
[INFO] |  |  +- (org.glassfish.jersey.core:jersey-common:jar:2.27:compile - omitted for duplicate)
[INFO] |  |  \- (org.glassfish.hk2.external:javax.inject:jar:2.5.0-b42:compile - omitted for duplicate)
[INFO] |  +- org.glassfish.jersey.inject:jersey-hk2:jar:2.27:compile
[INFO] |  |  +- (org.glassfish.jersey.core:jersey-common:jar:2.27:compile - omitted for duplicate)
[INFO] |  |  \- org.glassfish.hk2:hk2-locator:jar:2.5.0-b42:compile
[INFO] |  |     +- (org.glassfish.hk2.external:javax.inject:jar:2.5.0-b42:compile - omitted for duplicate)
[INFO] |  |     +- org.glassfish.hk2.external:aopalliance-repackaged:jar:2.5.0-b42:compile
[INFO] |  |     +- org.glassfish.hk2:hk2-api:jar:2.5.0-b42:compile
[INFO] |  |     |  +- javax.inject:javax.inject:jar:1:compile
[INFO] |  |     |  +- (org.glassfish.hk2:hk2-utils:jar:2.5.0-b42:compile - omitted for duplicate)
[INFO] |  |     |  \- (org.glassfish.hk2.external:aopalliance-repackaged:jar:2.5.0-b42:compile - omitted for duplicate) 
[INFO] |  |     +- org.glassfish.hk2:hk2-utils:jar:2.5.0-b42:compile
[INFO] |  |     |  +- (javax.annotation:javax.annotation-api:jar:1.2:compile - omitted for duplicate)
[INFO] |  |     |  \- (javax.inject:javax.inject:jar:1:compile - omitted for duplicate)
[INFO] |  |     +- (javax.annotation:javax.annotation-api:jar:1.2:compile - omitted for duplicate)
[INFO] |  |     \- org.javassist:javassist:jar:3.22.0-CR2:compile
[INFO] |  +- javax.ws.rs:javax.ws.rs-api:jar:2.1:compile
[INFO] |  +- javax.ws.rs:jsr311-api:jar:1.1.1:compile
[INFO] |  +- org.glassfish:javax.json:jar:1.0.4:compile
[INFO] |  +- com.google.guava:guava:jar:32.1.3-jre:compile
[INFO] |  |  +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO] |  |  +- com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:compile
[INFO] |  |  +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO] |  |  +- org.checkerframework:checker-qual:jar:3.37.0:compile
[INFO] |  |  +- com.google.errorprone:error_prone_annotations:jar:2.21.1:compile
[INFO] |  |  \- com.google.j2objc:j2objc-annotations:jar:2.8:compile
[INFO] |  +- com.google.collections:google-collections:jar:1.0:compile
[INFO] |  +- javax.activation:activation:jar:1.1:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.15.3:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.15.3:compile
[INFO] |  |  +- (com.fasterxml.jackson.core:jackson-annotations:jar:2.15.3:compile - omitted for duplicate)
[INFO] |  |  \- com.fasterxml.jackson.core:jackson-core:jar:2.15.3:compile
[INFO] |  \- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.15.3:compile
[INFO] |     +- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.15.3:compile
[INFO] |     |  +- (com.fasterxml.jackson.core:jackson-core:jar:2.15.3:compile - omitted for duplicate)
[INFO] |     |  \- (com.fasterxml.jackson.core:jackson-databind:jar:2.15.3:compile - omitted for duplicate)
[INFO] |     \- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.15.3:compile
[INFO] |        +- (com.fasterxml.jackson.core:jackson-annotations:jar:2.15.3:compile - omitted for duplicate)
[INFO] |        +- (com.fasterxml.jackson.core:jackson-core:jar:2.15.3:compile - omitted for duplicate)
[INFO] |        +- (com.fasterxml.jackson.core:jackson-databind:jar:2.15.3:compile - omitted for duplicate)
[INFO] |        +- jakarta.xml.bind:jakarta.xml.bind-api:jar:2.3.3:compile
[INFO] |        |  \- (jakarta.activation:jakarta.activation-api:jar:1.2.2:compile - omitted for duplicate)
[INFO] |        \- jakarta.activation:jakarta.activation-api:jar:1.2.2:compile
[INFO] \- junit:junit:jar:4.13.1:test
[INFO]    \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.762 s
[INFO] Finished at: 2024-01-09T10:54:37+08:00
[INFO] ------------------------------------------------------------------------
mykaul commented 9 months ago

That looks clean now, thanks.

tchaikov commented 8 months ago

@scylladb/scylla-jmx-maint hello maintainers, could you help merge this change?

denesb commented 8 months ago

Submodule update: scylladb/scylladb@5981900dca4da0de3f1072061296041f83b410cf