ARROW-5877: [FlightRPC] Fix Python<->Java auth issues - #4838

Closed
ghost wants to merge 1 commit into
masterfrom
unknown repository
Closed

ARROW-5877: [FlightRPC] Fix Python<->Java auth issues#4838
ghost wants to merge 1 commit into
masterfrom
unknown repository

Conversation

@ghost

@ghostghost commented Jul 9, 2019

Copy link
Copy Markdown
  • Java servers before didn't actually wait for the Handshake RPC to complete
  • Java servers didn't interrupt auth handlers if the client sent an error
  • Python/C++ clients didn't explicitly finish their end of the connection

Together, this led to the 'hanging forever' issue @rymurr saw.

I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).

Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510

@codecov-io

codecov-io commented Jul 9, 2019

Copy link
Copy Markdown

Codecov Report

Merging #4838 into master will increase coverage by 6.45%.
The diff coverage is 66.66%.

Impacted file tree graph

@@ Coverage Diff @@## master #4838 +/- ##
==========================================
+ Coverage 82.62% 89.08% +6.45% 
==========================================
Files 335 720 +385 Lines 43377 100165 +56788 Branches 1418 0 -1418 ==========================================
+ Hits 35841 89230 +53389 - Misses 7174 10935 +3761 + Partials 362 0 -362
Impacted FilesCoverage Δ
cpp/src/arrow/flight/client.cc92.13% <66.66%> (ø)
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/util/test_common/rand_gen.rs
js/src/builder/index.ts
... and 930 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7838886...fc35d19. Read the comment docs.

@rymurr

rymurr commented Jul 10, 2019

Copy link
Copy Markdown
Contributor

fwiw looks good to me! 👍

wesm
wesm approved these changes Jul 10, 2019
View reviewed changes

@wesmwesm left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. What's the path to having simulated auth on the integration test side?

@wesmwesm closed this in aa0631aJul 10, 2019
@ghost

ghost commented Jul 10, 2019

Copy link
Copy Markdown
Author

My plan is to refactor the integration script, or create a new one, to run tests that aren't based around Arrow datasets, and then add cases to test auth, error propagation, cancellation, and other features thoroughly. As part of that, or as a prerequisite, I may implement a common HTTP basic auth method for C++/Python. I will track the work in ARROW-5875.

@rymurr

rymurr commented Jul 10, 2019

Copy link
Copy Markdown
Contributor

Shout if you want a hand with that @lihalite, I had planned on exposing the BasicAuth protobuf that Java already uses into c++/python but if you had a more complete solution in mind I would be happy to help.

@ghost

ghost commented Jul 10, 2019

Copy link
Copy Markdown
Author

Ah if you already were planning on that, please go ahead! I filed a JIRA for that specifically, ARROW-5876.

Not sure when I can get to the testing - I want to have it well before 1.0 but I'm currently busy with internal projects.

wesm pushed a commit that referenced this pull request Jul 13, 2019
@lidavidm@wesm
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
5f9e2ff
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closes#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
@asfimportasfimport mentioned this pull request Aug 1, 2019
Closed
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
@lidavidm@kou
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
4a863c4
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closesapache#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-io@rymurr@wesm@lidavidm
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues - #4838
Closed
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Closed
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues#4838
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Conversation
@ghost
@ghostghost
commented
Jul 9, 2019

Copy link


Copy Markdown

  • Java servers before didn't actually wait for the Handshake RPC to complete
  • Java servers didn't interrupt auth handlers if the client sent an error
  • Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572

AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
@codecov-io
codecov-io
commented
Jul 9, 2019

Copy link


Copy Markdown

Codecov Report
Merging #4838 into master will increase coverage by 6.45%.

The diff coverage is 66.66%.
Impacted file tree graph
@@ Coverage Diff @@## master #4838 +/- ##
==========================================
+ Coverage 82.62% 89.08% +6.45% 
==========================================
Files 335 720 +385 Lines 43377 100165 +56788 Branches 1418 0 -1418 ==========================================
+ Hits 35841 89230 +53389 - Misses 7174 10935 +3761 + Partials 362 0 -362
Impacted FilesCoverage Δ
cpp/src/arrow/flight/client.cc92.13% <66.66%> (ø)
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/util/test_common/rand_gen.rs
js/src/builder/index.ts
... and 930 more
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update 7838886...fc35d19. Read the comment docs.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
fwiw looks good to me! 👍
wesm
wesm
approved these changes
Jul 10, 2019
View reviewed changes
@wesmwesm
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. What's the path to having simulated auth on the integration test side?
@wesmwesm
closed this
in
aa0631aJul 10, 2019
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
My plan is to refactor the integration script, or create a new one, to run tests that aren't based around Arrow datasets, and then add cases to test auth, error propagation, cancellation, and other features thoroughly. As part of that, or as a prerequisite, I may implement a common HTTP basic auth method for C++/Python. I will track the work in ARROW-5875.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
Shout if you want a hand with that @lihalite, I had planned on exposing the BasicAuth protobuf that Java already uses into c++/python but if you had a more complete solution in mind I would be happy to help.
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
Ah if you already were planning on that, please go ahead! I filed a JIRA for that specifically, ARROW-5876.
Not sure when I can get to the testing - I want to have it well before 1.0 but I'm currently busy with internal projects.
wesm pushed a commit
that referenced
this pull request
Jul 13, 2019
@lidavidm@wesm
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
5f9e2ff
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closes#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
@asfimportasfimport
mentioned this pull request
Aug 1, 2019

Closed

pribor pushed a commit
to GlobalWebIndex/arrow
that referenced
this pull request
Oct 24, 2025
@lidavidm@kou
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
4a863c4
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closesapache#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-io@rymurr@wesm@lidavidm

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues - #4838
Closed
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Closed
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues#4838
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Conversation
@ghost
@ghostghost
commented
Jul 9, 2019

Copy link


Copy Markdown

  • Java servers before didn't actually wait for the Handshake RPC to complete
  • Java servers didn't interrupt auth handlers if the client sent an error
  • Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572

AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
@codecov-io
codecov-io
commented
Jul 9, 2019

Copy link


Copy Markdown

Codecov Report
Merging #4838 into master will increase coverage by 6.45%.

The diff coverage is 66.66%.
Impacted file tree graph
@@ Coverage Diff @@## master #4838 +/- ##
==========================================
+ Coverage 82.62% 89.08% +6.45% 
==========================================
Files 335 720 +385 Lines 43377 100165 +56788 Branches 1418 0 -1418 ==========================================
+ Hits 35841 89230 +53389 - Misses 7174 10935 +3761 + Partials 362 0 -362
Impacted FilesCoverage Δ
cpp/src/arrow/flight/client.cc92.13% <66.66%> (ø)
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/util/test_common/rand_gen.rs
js/src/builder/index.ts
... and 930 more
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update 7838886...fc35d19. Read the comment docs.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
fwiw looks good to me! 👍
wesm
wesm
approved these changes
Jul 10, 2019
View reviewed changes
@wesmwesm
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. What's the path to having simulated auth on the integration test side?
@wesmwesm
closed this
in
aa0631aJul 10, 2019
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
My plan is to refactor the integration script, or create a new one, to run tests that aren't based around Arrow datasets, and then add cases to test auth, error propagation, cancellation, and other features thoroughly. As part of that, or as a prerequisite, I may implement a common HTTP basic auth method for C++/Python. I will track the work in ARROW-5875.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
Shout if you want a hand with that @lihalite, I had planned on exposing the BasicAuth protobuf that Java already uses into c++/python but if you had a more complete solution in mind I would be happy to help.
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
Ah if you already were planning on that, please go ahead! I filed a JIRA for that specifically, ARROW-5876.
Not sure when I can get to the testing - I want to have it well before 1.0 but I'm currently busy with internal projects.
wesm pushed a commit
that referenced
this pull request
Jul 13, 2019
@lidavidm@wesm
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
5f9e2ff
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closes#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
@asfimportasfimport
mentioned this pull request
Aug 1, 2019

Closed

pribor pushed a commit
to GlobalWebIndex/arrow
that referenced
this pull request
Oct 24, 2025
@lidavidm@kou
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
4a863c4
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closesapache#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-io@rymurr@wesm@lidavidm

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues - #4838
Closed
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Closed
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues#4838
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Conversation
@ghost
@ghostghost
commented
Jul 9, 2019

Copy link


Copy Markdown

  • Java servers before didn't actually wait for the Handshake RPC to complete
  • Java servers didn't interrupt auth handlers if the client sent an error
  • Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572

AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
@codecov-io
codecov-io
commented
Jul 9, 2019

Copy link


Copy Markdown

Codecov Report
Merging #4838 into master will increase coverage by 6.45%.

The diff coverage is 66.66%.
Impacted file tree graph
@@ Coverage Diff @@## master #4838 +/- ##
==========================================
+ Coverage 82.62% 89.08% +6.45% 
==========================================
Files 335 720 +385 Lines 43377 100165 +56788 Branches 1418 0 -1418 ==========================================
+ Hits 35841 89230 +53389 - Misses 7174 10935 +3761 + Partials 362 0 -362
Impacted FilesCoverage Δ
cpp/src/arrow/flight/client.cc92.13% <66.66%> (ø)
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/util/test_common/rand_gen.rs
js/src/builder/index.ts
... and 930 more
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update 7838886...fc35d19. Read the comment docs.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
fwiw looks good to me! 👍
wesm
wesm
approved these changes
Jul 10, 2019
View reviewed changes
@wesmwesm
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. What's the path to having simulated auth on the integration test side?
@wesmwesm
closed this
in
aa0631aJul 10, 2019
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
My plan is to refactor the integration script, or create a new one, to run tests that aren't based around Arrow datasets, and then add cases to test auth, error propagation, cancellation, and other features thoroughly. As part of that, or as a prerequisite, I may implement a common HTTP basic auth method for C++/Python. I will track the work in ARROW-5875.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
Shout if you want a hand with that @lihalite, I had planned on exposing the BasicAuth protobuf that Java already uses into c++/python but if you had a more complete solution in mind I would be happy to help.
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
Ah if you already were planning on that, please go ahead! I filed a JIRA for that specifically, ARROW-5876.
Not sure when I can get to the testing - I want to have it well before 1.0 but I'm currently busy with internal projects.
wesm pushed a commit
that referenced
this pull request
Jul 13, 2019
@lidavidm@wesm
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
5f9e2ff
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closes#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
@asfimportasfimport
mentioned this pull request
Aug 1, 2019

Closed

pribor pushed a commit
to GlobalWebIndex/arrow
that referenced
this pull request
Oct 24, 2025
@lidavidm@kou
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
4a863c4
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closesapache#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-io@rymurr@wesm@lidavidm

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues - #4838
Closed
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Closed
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues#4838
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Conversation
@ghost
@ghostghost
commented
Jul 9, 2019

Copy link


Copy Markdown

  • Java servers before didn't actually wait for the Handshake RPC to complete
  • Java servers didn't interrupt auth handlers if the client sent an error
  • Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572

AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
@codecov-io
codecov-io
commented
Jul 9, 2019

Copy link


Copy Markdown

Codecov Report
Merging #4838 into master will increase coverage by 6.45%.

The diff coverage is 66.66%.
Impacted file tree graph
@@ Coverage Diff @@## master #4838 +/- ##
==========================================
+ Coverage 82.62% 89.08% +6.45% 
==========================================
Files 335 720 +385 Lines 43377 100165 +56788 Branches 1418 0 -1418 ==========================================
+ Hits 35841 89230 +53389 - Misses 7174 10935 +3761 + Partials 362 0 -362
Impacted FilesCoverage Δ
cpp/src/arrow/flight/client.cc92.13% <66.66%> (ø)
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/util/test_common/rand_gen.rs
js/src/builder/index.ts
... and 930 more
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update 7838886...fc35d19. Read the comment docs.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
fwiw looks good to me! 👍
wesm
wesm
approved these changes
Jul 10, 2019
View reviewed changes
@wesmwesm
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. What's the path to having simulated auth on the integration test side?
@wesmwesm
closed this
in
aa0631aJul 10, 2019
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
My plan is to refactor the integration script, or create a new one, to run tests that aren't based around Arrow datasets, and then add cases to test auth, error propagation, cancellation, and other features thoroughly. As part of that, or as a prerequisite, I may implement a common HTTP basic auth method for C++/Python. I will track the work in ARROW-5875.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
Shout if you want a hand with that @lihalite, I had planned on exposing the BasicAuth protobuf that Java already uses into c++/python but if you had a more complete solution in mind I would be happy to help.
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
Ah if you already were planning on that, please go ahead! I filed a JIRA for that specifically, ARROW-5876.
Not sure when I can get to the testing - I want to have it well before 1.0 but I'm currently busy with internal projects.
wesm pushed a commit
that referenced
this pull request
Jul 13, 2019
@lidavidm@wesm
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
5f9e2ff
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closes#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
@asfimportasfimport
mentioned this pull request
Aug 1, 2019

Closed

pribor pushed a commit
to GlobalWebIndex/arrow
that referenced
this pull request
Oct 24, 2025
@lidavidm@kou
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
4a863c4
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closesapache#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-io@rymurr@wesm@lidavidm

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues - #4838
Closed
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Closed
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues#4838
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Conversation
@ghost
@ghostghost
commented
Jul 9, 2019

Copy link


Copy Markdown

  • Java servers before didn't actually wait for the Handshake RPC to complete
  • Java servers didn't interrupt auth handlers if the client sent an error
  • Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572

AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
@codecov-io
codecov-io
commented
Jul 9, 2019

Copy link


Copy Markdown

Codecov Report
Merging #4838 into master will increase coverage by 6.45%.

The diff coverage is 66.66%.
Impacted file tree graph
@@ Coverage Diff @@## master #4838 +/- ##
==========================================
+ Coverage 82.62% 89.08% +6.45% 
==========================================
Files 335 720 +385 Lines 43377 100165 +56788 Branches 1418 0 -1418 ==========================================
+ Hits 35841 89230 +53389 - Misses 7174 10935 +3761 + Partials 362 0 -362
Impacted FilesCoverage Δ
cpp/src/arrow/flight/client.cc92.13% <66.66%> (ø)
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/util/test_common/rand_gen.rs
js/src/builder/index.ts
... and 930 more
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update 7838886...fc35d19. Read the comment docs.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
fwiw looks good to me! 👍
wesm
wesm
approved these changes
Jul 10, 2019
View reviewed changes
@wesmwesm
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. What's the path to having simulated auth on the integration test side?
@wesmwesm
closed this
in
aa0631aJul 10, 2019
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
My plan is to refactor the integration script, or create a new one, to run tests that aren't based around Arrow datasets, and then add cases to test auth, error propagation, cancellation, and other features thoroughly. As part of that, or as a prerequisite, I may implement a common HTTP basic auth method for C++/Python. I will track the work in ARROW-5875.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
Shout if you want a hand with that @lihalite, I had planned on exposing the BasicAuth protobuf that Java already uses into c++/python but if you had a more complete solution in mind I would be happy to help.
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
Ah if you already were planning on that, please go ahead! I filed a JIRA for that specifically, ARROW-5876.
Not sure when I can get to the testing - I want to have it well before 1.0 but I'm currently busy with internal projects.
wesm pushed a commit
that referenced
this pull request
Jul 13, 2019
@lidavidm@wesm
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
5f9e2ff
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closes#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
@asfimportasfimport
mentioned this pull request
Aug 1, 2019

Closed

pribor pushed a commit
to GlobalWebIndex/arrow
that referenced
this pull request
Oct 24, 2025
@lidavidm@kou
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
4a863c4
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closesapache#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-io@rymurr@wesm@lidavidm

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues - #4838
Closed
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Closed
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues#4838
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Conversation
@ghost
@ghostghost
commented
Jul 9, 2019

Copy link


Copy Markdown

  • Java servers before didn't actually wait for the Handshake RPC to complete
  • Java servers didn't interrupt auth handlers if the client sent an error
  • Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572

AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
@codecov-io
codecov-io
commented
Jul 9, 2019

Copy link


Copy Markdown

Codecov Report
Merging #4838 into master will increase coverage by 6.45%.

The diff coverage is 66.66%.
Impacted file tree graph
@@ Coverage Diff @@## master #4838 +/- ##
==========================================
+ Coverage 82.62% 89.08% +6.45% 
==========================================
Files 335 720 +385 Lines 43377 100165 +56788 Branches 1418 0 -1418 ==========================================
+ Hits 35841 89230 +53389 - Misses 7174 10935 +3761 + Partials 362 0 -362
Impacted FilesCoverage Δ
cpp/src/arrow/flight/client.cc92.13% <66.66%> (ø)
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/util/test_common/rand_gen.rs
js/src/builder/index.ts
... and 930 more
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update 7838886...fc35d19. Read the comment docs.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
fwiw looks good to me! 👍
wesm
wesm
approved these changes
Jul 10, 2019
View reviewed changes
@wesmwesm
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. What's the path to having simulated auth on the integration test side?
@wesmwesm
closed this
in
aa0631aJul 10, 2019
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
My plan is to refactor the integration script, or create a new one, to run tests that aren't based around Arrow datasets, and then add cases to test auth, error propagation, cancellation, and other features thoroughly. As part of that, or as a prerequisite, I may implement a common HTTP basic auth method for C++/Python. I will track the work in ARROW-5875.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
Shout if you want a hand with that @lihalite, I had planned on exposing the BasicAuth protobuf that Java already uses into c++/python but if you had a more complete solution in mind I would be happy to help.
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
Ah if you already were planning on that, please go ahead! I filed a JIRA for that specifically, ARROW-5876.
Not sure when I can get to the testing - I want to have it well before 1.0 but I'm currently busy with internal projects.
wesm pushed a commit
that referenced
this pull request
Jul 13, 2019
@lidavidm@wesm
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
5f9e2ff
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closes#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
@asfimportasfimport
mentioned this pull request
Aug 1, 2019

Closed

pribor pushed a commit
to GlobalWebIndex/arrow
that referenced
this pull request
Oct 24, 2025
@lidavidm@kou
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
4a863c4
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closesapache#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-io@rymurr@wesm@lidavidm

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues - #4838
Closed
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Closed
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues#4838
ghost wants to merge 1 commit into
masterfrom 
unknown repository
Conversation
@ghost
@ghostghost
commented
Jul 9, 2019

Copy link


Copy Markdown

  • Java servers before didn't actually wait for the Handshake RPC to complete
  • Java servers didn't interrupt auth handlers if the client sent an error
  • Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572

AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
@codecov-io
codecov-io
commented
Jul 9, 2019

Copy link


Copy Markdown

Codecov Report
Merging #4838 into master will increase coverage by 6.45%.

The diff coverage is 66.66%.
Impacted file tree graph
@@ Coverage Diff @@## master #4838 +/- ##
==========================================
+ Coverage 82.62% 89.08% +6.45% 
==========================================
Files 335 720 +385 Lines 43377 100165 +56788 Branches 1418 0 -1418 ==========================================
+ Hits 35841 89230 +53389 - Misses 7174 10935 +3761 + Partials 362 0 -362
Impacted FilesCoverage Δ
cpp/src/arrow/flight/client.cc92.13% <66.66%> (ø)
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/util/test_common/rand_gen.rs
js/src/builder/index.ts
... and 930 more
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update 7838886...fc35d19. Read the comment docs.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
fwiw looks good to me! 👍
wesm
wesm
approved these changes
Jul 10, 2019
View reviewed changes
@wesmwesm
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. What's the path to having simulated auth on the integration test side?
@wesmwesm
closed this
in
aa0631aJul 10, 2019
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
My plan is to refactor the integration script, or create a new one, to run tests that aren't based around Arrow datasets, and then add cases to test auth, error propagation, cancellation, and other features thoroughly. As part of that, or as a prerequisite, I may implement a common HTTP basic auth method for C++/Python. I will track the work in ARROW-5875.
@rymurr
rymurr
commented
Jul 10, 2019

Copy link


Copy Markdown

Contributor
Shout if you want a hand with that @lihalite, I had planned on exposing the BasicAuth protobuf that Java already uses into c++/python but if you had a more complete solution in mind I would be happy to help.
@ghost
ghost
commented
Jul 10, 2019

Copy link


Copy Markdown

Author
Ah if you already were planning on that, please go ahead! I filed a JIRA for that specifically, ARROW-5876.
Not sure when I can get to the testing - I want to have it well before 1.0 but I'm currently busy with internal projects.
wesm pushed a commit
that referenced
this pull request
Jul 13, 2019
@lidavidm@wesm
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
5f9e2ff
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closes#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
@asfimportasfimport
mentioned this pull request
Aug 1, 2019

Closed

pribor pushed a commit
to GlobalWebIndex/arrow
that referenced
this pull request
Oct 24, 2025
@lidavidm@kou
ARROW-5877: [FlightRPC] Fix Python<->Java auth issues
4a863c4
- Java servers before didn't actually wait for the Handshake RPC to complete
- Java servers didn't interrupt auth handlers if the client sent an error
- Python/C++ clients didn't explicitly finish their end of the connection
Together, this led to the 'hanging forever' issue @rymurr saw.
I've left some TODOs as I would like to raise Flight-specific exceptions (which I'm working on in parallel).
Travis: https://travis-ci.com/lihalite/arrow/builds/118503572
AppVeyor: https://ci.appveyor.com/project/lihalite/arrow/builds/25858510
Author: David Li <li.davidm96@gmail.com>
Closesapache#4838 from lihalite/arrow-5877 and squashes the following commits:
fc35d19 <David Li> Wait for authentication to complete server-side
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-io@rymurr@wesm@lidavidm