You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "jduo (via GitHub)" <gi...@apache.org> on 2023/11/03 23:29:34 UTC
[PR] GH-38576: [Java] Change JDBC driver to optionally preserve cookies and auth tokens when getting streams [arrow]
jduo opened a new pull request, #38580:
URL: https://github.com/apache/arrow/pull/38580
### Rationale for this change
This change restores the original behavior of transmitting existing cookies and auth tokens when getting separate
streams returned by getFlightInfo after adding support for multiple endpoints.
These properties are now optional though.
### What changes are included in this PR?
- Change the JDBC driver to add new properties "retainCookies" and "retainAuth"
- These properties allow internally spawned connections for getting streams to use the cookies and bearer tokens from the original connection.
- Add tests for validating defaults from ArrowFlightSqlClient.Builder
### Are these changes tested?
Unit tests have been added.
### Are there any user-facing changes?
Yes. There are now properties and they are documented.
--
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-38576: [Java] Change JDBC driver to optionally preserve cookies and auth tokens when getting streams [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on code in PR #38580:
URL: https://github.com/apache/arrow/pull/38580#discussion_r1383974001
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java:
##########
@@ -672,10 +733,17 @@ public ArrowFlightSqlClientHandler build() throws SQLException {
client = clientBuilder.build();
final ArrayList<CallOption> credentialOptions = new ArrayList<>();
- if (authFactory != null) {
- credentialOptions.add(
- ClientAuthenticationUtils.getAuthenticate(
- client, username, password, authFactory, options.toArray(new CallOption[0])));
+ if (username != null && token == null) {
Review Comment:
That's correct. Thanks!
--
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-38576: [Java] Change JDBC driver to optionally preserve cookies and auth tokens when getting streams [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #38580:
URL: https://github.com/apache/arrow/pull/38580
--
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-38576: [Java] Change JDBC driver to optionally preserve cookies and auth tokens when getting streams [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38580:
URL: https://github.com/apache/arrow/pull/38580#discussion_r1383910174
##########
java/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java:
##########
@@ -672,10 +733,17 @@ public ArrowFlightSqlClientHandler build() throws SQLException {
client = clientBuilder.build();
final ArrayList<CallOption> credentialOptions = new ArrayList<>();
- if (authFactory != null) {
- credentialOptions.add(
- ClientAuthenticationUtils.getAuthenticate(
- client, username, password, authFactory, options.toArray(new CallOption[0])));
+ if (username != null && token == null) {
Review Comment:
Should this also use `isUsingUserPasswordAuth`?
--
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-38576: [Java] Change JDBC driver to optionally preserve cookies and auth tokens when getting streams [arrow]
Posted by "jduo (via GitHub)" <gi...@apache.org>.
jduo commented on PR #38580:
URL: https://github.com/apache/arrow/pull/38580#issuecomment-1793237407
@lidavidm @aiguofer
--
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-38576: [Java] Change JDBC driver to optionally preserve cookies and auth tokens when getting streams [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38580:
URL: https://github.com/apache/arrow/pull/38580#issuecomment-1801365480
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit fafd48c3196b4e6af0c09f633c6a3440419bf1f0.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/18475164080) has more details. It also includes information about 2 possible false positives 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