Copilot commented on code in PR #882:
URL: https://github.com/apache/ranger/pull/882#discussion_r2969240212
##########
hive-agent/pom.xml:
##########
@@ -201,6 +201,12 @@
<groupId>org.apache.ranger</groupId>
<artifactId>ranger-plugins-common</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
Adding this exclusion removes the `javax.ws.rs` API jar that
`ranger-plugins-common`/Jersey 1.x relies on, and this module doesn’t add an
alternative `javax.ws.rs` API dependency. This can lead to missing
`javax.ws.rs.*` classes at runtime. Remove the exclusion or add an explicit
JAX-RS API dependency (e.g., `jsr311-api` via `${jsr311-api.version}`).
```suggestion
```
##########
plugin-kafka/pom.xml:
##########
@@ -90,6 +90,12 @@
<groupId>org.apache.ranger</groupId>
<artifactId>ranger-plugins-common</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </exclusion>
+ </exclusions>
</dependency>
<dependency>
Review Comment:
This exclusion removes `javax.ws.rs:jsr311-api` from
`ranger-plugins-common`, but this module doesn’t declare a replacement
`javax.ws.rs` API dependency. Since `ranger-plugins-common` (Jersey 1.x REST
client) uses `javax.ws.rs` types, the plugin can end up missing these classes
at runtime. Remove the exclusion or add an explicit JAX-RS API dependency.
```suggestion
<dependency>
<groupId>javax.ws.rs</groupId>
<artifactId>jsr311-api</artifactId>
<version>1.1.1</version>
</dependency>
<dependency>
```
##########
plugin-kms/pom.xml:
##########
@@ -55,6 +55,12 @@
<groupId>org.apache.ranger</groupId>
<artifactId>ranger-plugins-common</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
Excluding `javax.ws.rs:jsr311-api` from `ranger-plugins-common` here can
leave the plugin without any `javax.ws.rs.*` API on the classpath (since this
module doesn't add an alternative). Either remove the exclusion or add an
explicit JAX-RS API dependency (e.g., `javax.ws.rs:jsr311-api` with
`${jsr311-api.version}`).
```suggestion
```
##########
ugsync/pom.xml:
##########
@@ -68,6 +68,17 @@
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.sun.jersey</groupId>
+ <artifactId>jersey-bundle</artifactId>
+ <version>${jersey-bundle.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
<dependency>
Review Comment:
The newly added `jersey-bundle` dependency excludes
`javax.ws.rs:jsr311-api`, but ugsync code imports `javax.ws.rs.core.*` (e.g.,
Cookie/NewCookie). Add an explicit JAX-RS API dependency (e.g.,
`javax.ws.rs:jsr311-api` using `${jsr311-api.version}` or another compatible
`javax.ws.rs` API), or drop this exclusion so the module still compiles/runs.
```suggestion
<dependency>
<groupId>javax.ws.rs</groupId>
<artifactId>jsr311-api</artifactId>
<version>${jsr311-api.version}</version>
</dependency>
<dependency>
```
##########
ugsync/pom.xml:
##########
@@ -165,6 +176,12 @@
<groupId>org.apache.ranger</groupId>
<artifactId>ranger-plugins-common</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
`ranger-plugins-common` now provides the JAX-RS API transitively, but this
exclusion removes `jsr311-api` without adding a replacement dependency in this
module. Given ugsync uses Jersey 1.x + `javax.ws.rs` types, this can lead to
missing `javax.ws.rs.*` classes at compile/runtime. Remove the exclusion or add
an explicit JAX-RS API dependency.
```suggestion
```
##########
security-admin/pom.xml:
##########
@@ -539,6 +539,12 @@
<groupId>org.apache.ranger</groupId>
<artifactId>ranger-plugins-common</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
This new exclusion removes `javax.ws.rs:jsr311-api` from
`ranger-plugins-common`, but `security-admin` also excludes `jsr311-api` from
its Jersey dependencies. Unless another `javax.ws.rs` API jar is added, this
can cause missing `javax.ws.rs.*` classes at compile/runtime. Consider removing
this exclusion or adding an explicit `javax.ws.rs` API dependency.
```suggestion
```
##########
tagsync/pom.xml:
##########
@@ -257,6 +257,12 @@
<groupId>org.apache.ranger</groupId>
<artifactId>ranger-plugins-common</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
This exclusion drops `javax.ws.rs:jsr311-api` from `ranger-plugins-common`,
but tagsync already excludes `jsr311-api` from its Jersey deps. Without adding
an explicit JAX-RS API dependency, tagsync can end up missing `javax.ws.rs.*`
classes needed by Jersey/Ranger REST clients. Remove the exclusion or add an
explicit JAX-RS API dependency (e.g., `jsr311-api` via `${jsr311-api.version}`).
```suggestion
```
##########
hdfs-agent/pom.xml:
##########
@@ -133,6 +133,12 @@
<groupId>org.apache.ranger</groupId>
<artifactId>ranger-plugins-common</artifactId>
<version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>javax.ws.rs</groupId>
+ <artifactId>jsr311-api</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
Excluding `javax.ws.rs:jsr311-api` from `ranger-plugins-common` here can
remove the only `javax.ws.rs.*` API jar from the agent’s dependency graph.
Since Ranger’s Jersey 1.x REST client code references `javax.ws.rs` types, this
risks runtime `ClassNotFoundException`. Remove the exclusion or add an explicit
JAX-RS API dependency.
```suggestion
```
##########
distro/src/main/assembly/plugin-trino.xml:
##########
@@ -87,8 +85,9 @@
<include>org.apache.ranger:credentialbuilder</include>
<include>org.codehaus.woodstox:stax2-api</include>
<include>com.fasterxml.woodstox:woodstox-core</include>
- <include>com.sun.jersey:jersey-bundle</include>
- <include>com.sun.jersey:jersey-json</include>
+
<include>com.sun.jersey:jersey-client:jar:${jersey-bundle.version}</include>
+
<include>com.sun.jersey:jersey-core:jar:${jersey-bundle.version}</include>
+
<include>com.sun.jersey:jersey-json:jar:${jersey-bundle.version}</include>
<include>com.fasterxml.jackson.core:jackson-annotations:jar:${fasterxml.jackson.version}</include>
<include>com.fasterxml.jackson.core:jackson-core:jar:${fasterxml.jackson.version}</include>
<include>com.fasterxml.jackson.core:jackson-databind:jar:${fasterxml.jackson.version}</include>
Review Comment:
The Trino plugin assembly now includes `jersey-client`/`jersey-core` (and
`jersey-json`) but does not include `javax.ws.rs:jsr311-api`. Since the
assembly uses an explicit `<includes>` filter, the resulting tarball may not
contain the `javax.ws.rs.*` API classes required by Jersey 1.x. Add an explicit
include for `jsr311-api` using `${jsr311-api.version}`.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
