-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-38198: [Go] Fix AuthenticateBasicToken to be reliable behind proxies #38199
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm i wonder if there's an easy way we could try to put together something that tests this in some way. But for now, this looks good to me
… proxies (apache#38199) ### Rationale for this change Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies. ### What changes are included in this PR? Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation. <img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc"> ### Are these changes tested? I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears. ### Are there any user-facing changes? No * Closes: apache#38198 Authored-by: Phillip LeBlanc <phillip@leblanc.tech> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 96d09e1. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them. |
… proxies (apache#38199) ### Rationale for this change Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies. ### What changes are included in this PR? Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation. <img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc"> ### Are these changes tested? I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears. ### Are there any user-facing changes? No * Closes: apache#38198 Authored-by: Phillip LeBlanc <phillip@leblanc.tech> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
… proxies (apache#38199) ### Rationale for this change Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies. ### What changes are included in this PR? Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation. <img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc"> ### Are these changes tested? I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears. ### Are there any user-facing changes? No * Closes: apache#38198 Authored-by: Phillip LeBlanc <phillip@leblanc.tech> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
… proxies (apache#38199) ### Rationale for this change Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies. ### What changes are included in this PR? Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation. <img width="609" alt="Screenshot 2023-10-11 at 6 18 40 PM" src="https://github.com/apache/arrow/assets/879445/05e23c6a-0ff8-41fc-825b-8add7fe938bc"> ### Are these changes tested? I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears. ### Are there any user-facing changes? No * Closes: apache#38198 Authored-by: Phillip LeBlanc <phillip@leblanc.tech> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
Rationale for this change
Fixes a bug in the Go Flight client library that makes using AuthenticateBasicToken unreliable behind proxies.
What changes are included in this PR?
Closes the sending side of the bi-directional stream for the Flight Handshake RPC call before trying to read the headers. This matches the C++ implementation.
Are these changes tested?
I've tested these changes against my service deployed behind CloudFlare and verified the error listed in the linked issue disappears.
Are there any user-facing changes?
No