lidavidm commented on a change in pull request #1402:
URL: https://github.com/apache/arrow-rs/pull/1402#discussion_r820096763
##########
File path: integration-testing/src/flight_client_scenarios/integration_test.rs
##########
@@ -185,7 +185,8 @@ async fn consume_flight_location(
let mut location = location;
// The other Flight implementations use the `grpc+tcp` scheme, but the
Rust http libs
// don't recognize this as valid.
- location.uri = location.uri.replace("grpc+tcp://", "grpc://");
+ // more details: https://github.com/apache/arrow-rs/issues/1398
+ location.uri = location.uri.replace("grpc+tcp://", "http://");
Review comment:
Just some context on this bit: Flight's `Location` is never actually
directly used as a gRPC URI. The C++ and Java implementations take apart and
reconstruct the URI, so, grpc+tcp was never menat to be passed to gRPC. You can
see here:
https://github.com/apache/arrow/blob/4ef95eb89f9202dfcd9017633cf55671d56e337f/cpp/src/arrow/flight/client.cc#L935-L939
I do wonder if gRPC got more strict. The last passing run used gRPC 1.43.2,
the first failing run used gRPC 1.44.0. By my read, this is the responsible
commit, first seen in gRPC 1.44.0:
https://github.com/grpc/grpc/commit/0deb64d1f6461b96a65ed847e63251a54856654f
It adds the error message to hpack_parser.cc (see ReportMetadataParseError)
and crucially adds new validation to metadata_batch.h (see HttpSchemeMetadata).
So I guess this was new validation added in v1.44.0.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]