[Go][FlightRPC] Implement session management #40155
Comments
This was referenced Feb 20, 2024
|
I can work on this |
|
Thanks! FWIW, if you comment just "take" on an issue, a bot should assign it to you |
|
I've just opened a PR for this: #40284 |
zeroshade
pushed a commit
that referenced
this issue
Mar 1, 2024
…0284) ### Rationale for this change Brings Go implementation in parity with recent session management additions to Java and C++: #34865 ### What changes are included in this PR? - Go Flight/FlightSQL implementations of session management RPC handlers - Implementation of cookie-based session middleware - Implementation of stateful (id-lookup based) sessions/tokens - Implementation of stateless (fully encoded) sessions/tokens - Fix minor C++ logic bug when closing sessions - Update Java integration test server to return an empty session if `getSessionOptions` is called before `setSessionOptions` - Refactor of `DoAction` handlers to consolidate the code that is essentially copied between them. - As part of this I found an issue with `CancelFlightInfo` where a copy of the message was being returned instead of a pointer as is typically the case with `proto.Message`'s. I updated the return type and any usage throughout the code base as part of the refactor. ### Are these changes tested? Yes, both integration and unit tests are included. A few tests were added in the Go integration suite beyond the existing coverage in the Java/C++ suites. These tests aim to demonstrate my understanding of session semantics in those scenarios, please let me know if you believe the details are not accurate. Some of the new integration tests failed in the Java/C++ scenarios. I made very minor changes to those implementations to fix certain failures but there are still some remaining bugs (assuming these are testing the right semantics). Specifically: - The integration test for reopening a previously closed session passes on Go/Java, but fails for C++ so it is commented out. - This implementation prefers to set any cookies in the gRPC trailer which works fine for Go/C++, but not for Java. As a temporary workaround this implementation will _also_ set the cookie in the gRPC header if a new session was created. This is sufficient to maintain compatibility with Java stateful sessions where the session ID token can be known at the time of creation, but is not robust to other scenarios such as stateless sessions where in many cases the token cannot be known until after the RPC has completed. ### Are there any user-facing changes? Yes, session management RPC as well as middleware implementations are included. Functionality is entirely additive * GitHub Issue: #40155 Authored-by: joel <joellubi@gmail.com> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
|
Issue resolved by pull request 40284 |
thisisnic
pushed a commit
to thisisnic/arrow
that referenced
this issue
Mar 8, 2024
…nt (apache#40284) ### Rationale for this change Brings Go implementation in parity with recent session management additions to Java and C++: apache#34865 ### What changes are included in this PR? - Go Flight/FlightSQL implementations of session management RPC handlers - Implementation of cookie-based session middleware - Implementation of stateful (id-lookup based) sessions/tokens - Implementation of stateless (fully encoded) sessions/tokens - Fix minor C++ logic bug when closing sessions - Update Java integration test server to return an empty session if `getSessionOptions` is called before `setSessionOptions` - Refactor of `DoAction` handlers to consolidate the code that is essentially copied between them. - As part of this I found an issue with `CancelFlightInfo` where a copy of the message was being returned instead of a pointer as is typically the case with `proto.Message`'s. I updated the return type and any usage throughout the code base as part of the refactor. ### Are these changes tested? Yes, both integration and unit tests are included. A few tests were added in the Go integration suite beyond the existing coverage in the Java/C++ suites. These tests aim to demonstrate my understanding of session semantics in those scenarios, please let me know if you believe the details are not accurate. Some of the new integration tests failed in the Java/C++ scenarios. I made very minor changes to those implementations to fix certain failures but there are still some remaining bugs (assuming these are testing the right semantics). Specifically: - The integration test for reopening a previously closed session passes on Go/Java, but fails for C++ so it is commented out. - This implementation prefers to set any cookies in the gRPC trailer which works fine for Go/C++, but not for Java. As a temporary workaround this implementation will _also_ set the cookie in the gRPC header if a new session was created. This is sufficient to maintain compatibility with Java stateful sessions where the session ID token can be known at the time of creation, but is not robust to other scenarios such as stateless sessions where in many cases the token cannot be known until after the RPC has completed. ### Are there any user-facing changes? Yes, session management RPC as well as middleware implementations are included. Functionality is entirely additive * GitHub Issue: apache#40155 Authored-by: joel <joellubi@gmail.com> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the enhancement requested
The session management primitives from #34865 aren't yet supported in Go.
Component(s)
FlightRPC, Go
The text was updated successfully, but these errors were encountered: