You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "laurentgo (via GitHub)" <gi...@apache.org> on 2024/03/30 03:42:08 UTC
[PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
laurentgo opened a new pull request, #40908:
URL: https://github.com/apache/arrow/pull/40908
### Rationale for this change
FlightSQL JDBC Driver does not shade slfj4 api which may come into conflict into the version used by an application. If the application uses slf4j 1.x, it may cause the application slf4j backend to not be loaded properly.
The change configured maven-shade-plugin to also shade slf4j-api. To make sure log messages are still visible, slf4j-jdk14 is included as well so that all messages will be redirected to java.util.logging framework. The application can use jul-to-slf4j adapter to redirect log messages back to slf4j.
### What changes are included in this PR?
Overrides Driver#getParentLogger() to return the root logger for the JDBC driver (which is `org.apache.arrow.driver.jdbc`). To make sure shaded dependencies loggers are included as well, change relocation from cfjd. to org.apache.arrow.driver.jdbc.shaded. (or oaadj for native libraries)
### Are these changes tested?
Verifying that slf4j-api is shaded along with the other relocation changes are covered by `ITDriverJarValidation`
### Are there any user-facing changes?
Yes. Driver will not expose directly slf4j api and the logger names for the shaded dependencies have been updated. For applications which were relying on configuring directly a slf4j logging backend for the driver, they may need to include `org.slf4j:slf4-api` and `org.slf4j:jul-to-slf4j` for logging configuration to work.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
Posted by "laurentgo (via GitHub)" <gi...@apache.org>.
laurentgo commented on code in PR #40908:
URL: https://github.com/apache/arrow/pull/40908#discussion_r1545795292
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java:
##########
@@ -58,7 +59,7 @@ public class ArrowFlightJdbcDriver extends UnregisteredDriver {
// Netty requires some extra properties to unlock some native memory management api
// Setting this property if not already set externally
// This has to be done before any netty class is being loaded
- final String key = "cfjd.io.netty.tryReflectionSetAccessible";
+ final String key = "io.netty.tryReflectionSetAccessible";
Review Comment:
The main use of this property is to be set directly by the driver (not the users) in order for the driver to actually work. Checking if it is set sounds more like a debugging capability.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40908:
URL: https://github.com/apache/arrow/pull/40908#issuecomment-2027898737
:warning: GitHub issue #40907 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40908:
URL: https://github.com/apache/arrow/pull/40908#issuecomment-2027908813
Revision: 3008f79e06d9428d3f6685d1afa9753c1c53ba7b
Submitted crossbow builds: [ursacomputing/crossbow @ actions-b5deb1933b](https://github.com/ursacomputing/crossbow/branches/all?query=actions-b5deb1933b)
|Task|Status|
|----|------|
|java-jars|[](https://github.com/ursacomputing/crossbow/actions/runs/8488316619/job/23257164058)|
|verify-rc-source-java-linux-almalinux-8-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8488316602/job/23257163678)|
|verify-rc-source-java-linux-conda-latest-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8488316652/job/23257163788)|
|verify-rc-source-java-linux-ubuntu-20.04-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8488316621/job/23257163664)|
|verify-rc-source-java-linux-ubuntu-22.04-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8488316601/job/23257163672)|
|verify-rc-source-java-macos-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/8488316659/job/23257163794)|
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on PR #40908:
URL: https://github.com/apache/arrow/pull/40908#issuecomment-2027908391
@github-actions crossbow submit -g java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #40908:
URL: https://github.com/apache/arrow/pull/40908#discussion_r1545735634
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java:
##########
@@ -58,7 +59,7 @@ public class ArrowFlightJdbcDriver extends UnregisteredDriver {
// Netty requires some extra properties to unlock some native memory management api
// Setting this property if not already set externally
// This has to be done before any netty class is being loaded
- final String key = "cfjd.io.netty.tryReflectionSetAccessible";
+ final String key = "io.netty.tryReflectionSetAccessible";
Review Comment:
@lidavidm do you mean testing/shading both `cfjd` and regular shading ? I think it could be confusing for users. Just wondering.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #40908:
URL: https://github.com/apache/arrow/pull/40908#issuecomment-2033513971
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 42b49df0f3dc1586ad38c608ec93f382a4f4e3c4.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/23375785092) has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #40908:
URL: https://github.com/apache/arrow/pull/40908#discussion_r1545720259
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java:
##########
@@ -58,7 +59,7 @@ public class ArrowFlightJdbcDriver extends UnregisteredDriver {
// Netty requires some extra properties to unlock some native memory management api
// Setting this property if not already set externally
// This has to be done before any netty class is being loaded
- final String key = "cfjd.io.netty.tryReflectionSetAccessible";
+ final String key = "io.netty.tryReflectionSetAccessible";
Review Comment:
Do we want to support both the old and the new keys to minimize potential compatibility breaks?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40907: [Java][FlightSQL] Shade slf4j-api in JDBC driver [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #40908:
URL: https://github.com/apache/arrow/pull/40908
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org