[FlightRPC][JAVA][Draft] Bearer token refresh interceptor #5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jduo
reviewed
Feb 15, 2022
| @@ -0,0 +1,130 @@ | |||
| package org.apache.arrow.flight; | |||
jduo
reviewed
Feb 15, 2022
| @Override | ||
| public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(MethodDescriptor<ReqT, RespT> method, | ||
| io.grpc.CallOptions callOptions, Channel next) { | ||
| System.out.println("Intercept the call"); |
jduo
reviewed
Feb 15, 2022
| @@ -98,7 +98,9 @@ | |||
| this.middleware = middleware; | |||
|
|
|||
| final ClientInterceptor[] interceptors; | |||
| interceptors = new ClientInterceptor[]{authInterceptor, new ClientInterceptorAdapter(middleware)}; | |||
| interceptors = new ClientInterceptor[]{authInterceptor, new RefreshClientInterceptor(), new ClientInterceptorAdapter(middleware)}; | |||
There was a problem hiding this comment.
The interceptor should be optional based on FlightClient.Builder. How do we propagate how to do the refresh?
jduo
reviewed
Feb 15, 2022
| return status.getCode().toStatus() == Status.UNAUTHENTICATED; | ||
| } | ||
| } | ||
| } |
jduo
reviewed
Feb 15, 2022
|
|
||
| @Override | ||
| public void onClose(Status status, Metadata trailers) { | ||
| System.out.println("Run close method from listener interceptor"); |
There was a problem hiding this comment.
It is difficult to understand what's going on here.
It's especially hard to reason because it's using both recursion and asynchronous execution. This needs alot more explanation.
jduo
reviewed
Feb 15, 2022
| } | ||
|
|
||
| class CheckingListener extends ForwardingClientCallListener { | ||
| Listener<RespT> delegate; |
There was a problem hiding this comment.
It seems like this shouldn't be mutable state, but it is. Why does it change? Why is null sometimes a legal value but not all times?
jduo
reviewed
Feb 15, 2022
|
|
||
| @Override | ||
| public void cancel(@Nullable String message, @Nullable Throwable cause) { | ||
| System.out.println("Run cancel method from interceptor"); |
jduo
reviewed
Feb 15, 2022
| @Override | ||
| public void request(int numMessages) { | ||
| System.out.println("Run request method from interceptor"); | ||
| req += numMessages; |
There was a problem hiding this comment.
Why is this tracked but never used?
vfraga
added a commit
that referenced
this pull request
Mar 29, 2022
* Flight SQL Ratification Based On Community Feedback #7 (apache#98) * Remove scope from 'hamcrest' dependency on java/pom.xml * Use flight top-level module on parent pom.xml instead of declaring each one * Avoid using getStatement inside StatementContext methods * Make StatementContext.getQuery() return String * Minor fixes on pom.xml * Move 'os-maven-plugin' to parent pom.xml * Update protobuf generation on pom.xml files * Use ClassLoader#getResource to get network.properties on TestFlightSql * Bind to any ephemeral port on TestFlightSql * Move JDBC-Arrow type default conversion from JdbcToArrowConfig to JdbcToArrowUtils * Micro-optimization: initialize ArrayList with the right size * Fix null-check on PreparedStatement#setParameters * Avoid wrapping vector into a ImmutableList and then into an ArrayList on FlightSqlExample#getTablesRoot * Remove null-check on VectorSchemaRoot on FlightSqlClient#setParameters() * Remove the need of separate cache for ResultSets * Add missing 'final' modifiers * Fix maven build from different directories (apache#114) * Add CrossReference methods to SqlProducer * Clean-up: remove boilerplate code by replacing with tools provided by Arrow Flight JDBC Adapter * Fix checkstyle and dependency management errors * Ignore broken tests * Fix broken tests for CreatePreparedStatement * Fix Schema generation not setting an unknown column type to nullable * Change FlightSqlProducer from abstract class to interface * Refactor tests due to creation of new column of primaryKey * Nit: fix checkstyle * Fix leaking connections on connection pool * [WIP] FlightSQL Ratification based on Community Comments (apache#73) * Move FlightSql examples to their own subpackage * Fix checkstyle issues * fix: change Status use to CallStatus * Remove unnecessary overhead of wrapping nullable objects into Optionals for the sole purpose of null-checking * Replace Guava's Preconditions with the ones provided by Apache * Fix typo in FlightSql.proto * Fix ordering of schema for FlightSql.proto * Explain why reserved range of IDs for GetSqlInfo is not entirely in use * Add comment to CommandGetTables to explain the encoding of table_schema * Remove redundat information on schemas * Fixed Javadoc on some methods, added Thread interrupt to executeUpdate methods, and updated Signal exceptions to CallStatus with description * Replace int32 with uint32 for GetSqlInfo name representation * Replace AssertionError with StatusRuntimeException for whenever attempting to unpack an invalid protobuf message * add comment to FlightSql.proto to update_rule and delete_rule * Replace inconsistent exception handling with CallStatus predetermined exceptions * correct comment to CreatePreparedStatement on FlightSql.proto * Remove unused dependencies * fix: change Status use to CallStatus on FlightSqlProducer * Changed from if not null check to Objects requireNonNull on Flight SQL Client * Remove Nullable annotation * Changed from checkNotNull to Objects#requireNotNull with description on Flight SQL Example * Add CallOptions to every RPC call by the client * Fix Maven dependency problems and checkstyle violations * Replace generic Collections with Lists when order matters in an RPC call * Fix Javadoc for FlightSqlClient * Add description to StatusRuntimeExceptions * Add descriptions to Exceptions * Correct update_rule and delete_rule description on FlighSql.proto * Verify wheter Root is empty before sending request to server * Add call options to PreparedStatement * Replace constant checking of whether client is open with #checkOpen * Add CallOptions to #close for PreparedStatement * Refactor PreparedStatement usages of CallOptions * Fix broken tests * Fix FlightSql.proto documentation * Update documentation for format/FlightSql.proto Co-authored-by: kylep-dremio <38920967+kylep-dremio@users.noreply.github.com> * Fix checkstyle violations * Require non null tables for GetExportedKeys and GetImportedKeys * Not storing CallOptions in PreparedStatement * Update documentation comments for protobuf * Replace IntVector for UInt1Vector for delete_rule and update_rule * Fix protobuf for FlightSQL * Fix bug with empty metadata * Update update_rule and delete_rule documentation on proto * Remove explicit dependency on JDBC's DatabaseMetaData on UpdateDeleteRules * Use MessageOptions instead of FieldOptions on proto * Add missing JavaDoc about 'options' parameter * Fix CommandGetSqlInfo documentation * Add @throws to FlightSqlClient#checkOpen JavaDoc Co-authored-by: Juscelino Junior <juscelinojunior@id.uff.br> Co-authored-by: Vinicius Fraga <sxvinifp@gmail.com> Co-authored-by: Rafael Telles <rafael@telles.dev> Co-authored-by: kylep-dremio <38920967+kylep-dremio@users.noreply.github.com> * Create a module for keeping all Arrow Flight-related submodules * Fix checkstyle violations in Arrow Flight JDBC Driver * Create a module for keeping all Arrow Flight-related submodules * Fix checkstyle violations in Arrow Flight JDBC Driver * Fix Minor POM Issues to allow Maven build (apache#161) * Fix minor POM Issues to allow Maven Build * Revert Flight SQL POM State * Remove unused imports Co-authored-by: Rafael Telles <rafael@telles.dev> * Ignore broken tests * Refactor tests due to creation of new column of primaryKey * Create a module for keeping all Arrow Flight-related submodules * Fix checkstyle violations in Arrow Flight JDBC Driver * Create a module for keeping all Arrow Flight-related submodules * Fix checkstyle violations in Arrow Flight JDBC Driver * [FlightRPC] Flight SQL POC Add extensions in the Apache Arrow project’s Arrow Flight modules to provide a standard way for clients and servers to communicate with SQL-like semantics. Do not pull to master. A message to the mailing list will accompany this and another proposal in the coming days for discussion. * Update FlightSqlProducer to conform to new design. Rename files for SQL -> Sql. Correct compilation errors in client code, but design needs to be updated. Tests do not yet compile. * Correct the dense_union type for schema return of SQL info. Correct some additional SQL -> Sql file renames. Reduce the test compilation problems (still more to do). * Additional CR changes. Note - FlightSqlExample is not functional and needs to be updated. * Add support for primary and foreign keys. * Fix broken Maven build * Add Hamcrest as a root-level dependency * Create test for GetCatalogs * Update tests for GetTables * Fix test for GetTables * Enable support for includeSchema in GetTables * Fix checkstyle and dependency management errors * Remove boilerplate code for creating a new Schema by reusing default converter from JdbcToArrowConfig#DEFAULT_JDBC_TO_ARROW_TYPE_CONVERTER * WIP: Working on fixing data consistency issue where catalog is null in some parts and "" in others * Implement FlightSqlExample's GetPrimaryKey command * WIP: Add support for GetCatalogs: GetFlightInfoCatalogs * WIP: Add support for GetCatalogs: GetStreamCatalogs * WIP: Add support for GetTableTypes: getFlightInfoTableTypes * WIP: Add support for GetTableTypes: getStreamTableTypes * Fix rebase conflicts * Minor refactor: apply DRY principle to repeated methods * Ignore broken tests * Fix broken tests for CreatePreparedStatement * Fix Schema generation not setting an unknown column type to nullable * Change FlightSqlProducer from abstract class to interface * Update FlightSql protobuf to allow nullable values as parameters for nullable fields * Start separating commandForeignKey into two new commands * Fix CheckStyle issues * Change empty string to null value * Implement CommandGetImportedKeys * Fix checkstyle violations * Minor refactor: remove unused methods * Implement FlightSqlClient.executeUpdate * Refactor the code to not use string when getting from cache * Nit: fix checkstyle * Fix conflicts between tests for creating a new statement and checking its schema * Minor refactor: remove unused fields @ FlightSqlUtils * Fix wrong StreamListener usages and multiple instances of RootAllocators * Fix checkstyle * Modify execute preparedStatement flow * Fix leaking connections on connection pool * Deal with query with parameter in the preparedStatement * Fix rebase issues with FlightSql.proto * Fix missing code on adapter/jdbc/JdbcToArrowUtils.java due to rebase issue * [WIP] FlightSQL Ratification based on Community Comments (apache#73) * Move FlightSql examples to their own subpackage * Fix checkstyle issues * fix: change Status use to CallStatus * Remove unnecessary overhead of wrapping nullable objects into Optionals for the sole purpose of null-checking * Replace Guava's Preconditions with the ones provided by Apache * Fix typo in FlightSql.proto * Fix ordering of schema for FlightSql.proto * Explain why reserved range of IDs for GetSqlInfo is not entirely in use * Add comment to CommandGetTables to explain the encoding of table_schema * Remove redundat information on schemas * Fixed Javadoc on some methods, added Thread interrupt to executeUpdate methods, and updated Signal exceptions to CallStatus with description * Replace int32 with uint32 for GetSqlInfo name representation * Replace AssertionError with StatusRuntimeException for whenever attempting to unpack an invalid protobuf message * add comment to FlightSql.proto to update_rule and delete_rule * Replace inconsistent exception handling with CallStatus predetermined exceptions * correct comment to CreatePreparedStatement on FlightSql.proto * Remove unused dependencies * fix: change Status use to CallStatus on FlightSqlProducer * Changed from if not null check to Objects requireNonNull on Flight SQL Client * Remove Nullable annotation * Changed from checkNotNull to Objects#requireNotNull with description on Flight SQL Example * Add CallOptions to every RPC call by the client * Fix Maven dependency problems and checkstyle violations * Replace generic Collections with Lists when order matters in an RPC call * Fix Javadoc for FlightSqlClient * Add description to StatusRuntimeExceptions * Add descriptions to Exceptions * Correct update_rule and delete_rule description on FlighSql.proto * Verify wheter Root is empty before sending request to server * Add call options to PreparedStatement * Replace constant checking of whether client is open with #checkOpen * Add CallOptions to #close for PreparedStatement * Refactor PreparedStatement usages of CallOptions * Fix broken tests * Fix FlightSql.proto documentation * Update documentation for format/FlightSql.proto Co-authored-by: kylep-dremio <38920967+kylep-dremio@users.noreply.github.com> * Fix checkstyle violations * Require non null tables for GetExportedKeys and GetImportedKeys * Not storing CallOptions in PreparedStatement * Update documentation comments for protobuf * Replace IntVector for UInt1Vector for delete_rule and update_rule * Fix protobuf for FlightSQL * Fix bug with empty metadata * Update update_rule and delete_rule documentation on proto * Remove explicit dependency on JDBC's DatabaseMetaData on UpdateDeleteRules * Use MessageOptions instead of FieldOptions on proto * Add missing JavaDoc about 'options' parameter * Fix CommandGetSqlInfo documentation * Add @throws to FlightSqlClient#checkOpen JavaDoc Co-authored-by: Juscelino Junior <juscelinojunior@id.uff.br> Co-authored-by: Vinicius Fraga <sxvinifp@gmail.com> Co-authored-by: Rafael Telles <rafael@telles.dev> Co-authored-by: kylep-dremio <38920967+kylep-dremio@users.noreply.github.com> * FlightSQL Ratification based on Community Comments (round 2) (apache#85) * Remove unused client_execution_handler from Protobuf * Update documentation on CommandGetPrimaryKeys * Update documentation on CommandGetImportedKeys and CommandGetExportedKeys * Change exception type on FlightSqlClient#executeUpdate * Add @return to FlightSqlClient#executeUpdate JavaDoc * Switch order of key_name and key_sequence on CommandGetTableKeys documentation * Update JavaDoc for FlIghtSqlClient#clearParameters * Add private constructor to FlightSqlProducer.SqlInfo * Update JavaDoc for FlIghtSqlClient#clearParameters * Fix wrong CommandGetPrimaryKeys documentation on Proto file * Fix order of key_name and key_sequence * Split CommandStatementQuery in 2 messages, one for Command and other for Ticket * Fix leaking Connections on FlightSqlExample * Remove unused variable of cache * Flight SQL Ratification Based On Community Feedback #5 (apache#91) * Delegate GetSchemaImportedKeys * Remove schema retrieval methods for catalog functions and delegate to constants * Add IPC encapsulation to Schema serialization * Fix checkstyle violations * Update javadoc for FlightSqlClient * Update documentation for FlightSql.proto Co-authored-by: Abner Eduardo Ferreira <abenaru@protonmail.ch> * Flight SQL Ratification Based On Community Feedback #6 (apache#94) * Refactored FlightSql Statement Constant names * Defined non-nullable parameters for FlightSql proto * Resolved minimal checkstyle issues * Added further documentation for catalog and schema * Refactored FlightSql proto comments to include more information * Added Field/FieldType notNullable methods * Refactored FlightSqlClient and FlightSqlExample to leverage Field notNullable method * Removed opaque query warning from FlightSql proto * Added the optional tag for the returned schema of getTables to proto * Flight SQL Ratification Based On Community Feedback #7 (apache#98) * Remove scope from 'hamcrest' dependency on java/pom.xml * Use flight top-level module on parent pom.xml instead of declaring each one * Avoid using getStatement inside StatementContext methods * Make StatementContext.getQuery() return String * Minor fixes on pom.xml * Move 'os-maven-plugin' to parent pom.xml * Update protobuf generation on pom.xml files * Use ClassLoader#getResource to get network.properties on TestFlightSql * Bind to any ephemeral port on TestFlightSql * Move JDBC-Arrow type default conversion from JdbcToArrowConfig to JdbcToArrowUtils * Micro-optimization: initialize ArrayList with the right size * Fix null-check on PreparedStatement#setParameters * Avoid wrapping vector into a ImmutableList and then into an ArrayList on FlightSqlExample#getTablesRoot * Remove null-check on VectorSchemaRoot on FlightSqlClient#setParameters() * Remove the need of separate cache for ResultSets * Add missing 'final' modifiers * Fix missing generated sources on built flight-sql jar (apache#101) * Bump protobuf version * Flight SQL Ratification Based On Community Feedback #8 (apache#113) * Change scope of arrow-memory-netty to test for flight-sql * Remove unused dependency arrow-memory-netty * Update common-pool2 and common-dbcp2 dependencies * Remove 'executions' from parent pom.xml for plugin protobuf-maven-plugin * Adjust protobuf-maven-plugin settings on pom.xml files * Move dep.protobuf.version and dep.grpc.version to top pom.xml * Remove <url> from arrow-flight's pom.xml * Fix maven build from different directories (apache#114) * Add test cases for bitshifting operations required for filtering out some SqlInfo data * Fix Schema serialization and deserialization on Flight SQL methods * [FlightSQL] Add missing method for creating bitmask from GetSqlInfo option enum (apache#148) * Add util method for creating bitmask from multiple protobuf enums for FlightSql GetSqlInfo enum * Add test cases for utility method for creating bitmask from protobuf options * Make changes regarding to reviews Co-authored-by: Rafael Telles <rafael@telles.dev> * Create a module for keeping all Arrow Flight-related submodules * Fix checkstyle violations in Arrow Flight JDBC Driver * Create a module for keeping all Arrow Flight-related submodules * Fix checkstyle violations in Arrow Flight JDBC Driver * WIP: Refactor SqlInfoProvider * WIP * WIP * WIP * Fix rebase issues * Restore FlightSql.proto state * Fix checkstyle * Fix rebase colateral * Restore FlightSql.proto Co-authored-by: Jose Almeida <almeidajcr90@gmail.com> Co-authored-by: Abner Eduardo Ferreira <abenaru@protonmail.ch> Co-authored-by: Juscelino Junior <juscelinojunior@id.uff.br> Co-authored-by: Vinicius Fraga <sxvinifp@gmail.com> Co-authored-by: kylep-dremio <38920967+kylep-dremio@users.noreply.github.com> Co-authored-by: Vinicius Fraga <62815192+vfraga@users.noreply.github.com> Co-authored-by: Ryan Nicholson <rnicholson@dremio.com> Co-authored-by: Kyle Porter <kporter@dremio.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.