tustvold commented on code in PR #2586:
URL: https://github.com/apache/arrow-rs/pull/2586#discussion_r954736857
##########
arrow-flight/src/lib.rs:
##########
@@ -254,10 +254,17 @@ impl From<SchemaAsIpc<'_>> for FlightData {
}
}
-impl From<SchemaAsIpc<'_>> for SchemaResult {
- fn from(schema_ipc: SchemaAsIpc) -> Self {
- let IpcMessage(vals) = flight_schema_as_flatbuffer(schema_ipc.0,
schema_ipc.1);
- SchemaResult { schema: vals }
+impl TryFrom<SchemaAsIpc<'_>> for SchemaResult {
+ type Error = ArrowError;
+
+ fn try_from(schema_ipc: SchemaAsIpc) -> ArrowResult<Self> {
Review Comment:
My point is twofold:
* We clearly are missing a test as the change you have made to the protocol
should be failing a test somewhere
* We need to update the logic to interpret the `SchemaResult` which does
exist and following this PR will be incorrect
_I do really wonder why arrow flight is using opaque byte objects, the whole
point of using an API DSL is to avoid this but hey ho, this protocol is wild_
##########
arrow-flight/src/lib.rs:
##########
@@ -254,10 +254,17 @@ impl From<SchemaAsIpc<'_>> for FlightData {
}
}
-impl From<SchemaAsIpc<'_>> for SchemaResult {
- fn from(schema_ipc: SchemaAsIpc) -> Self {
- let IpcMessage(vals) = flight_schema_as_flatbuffer(schema_ipc.0,
schema_ipc.1);
- SchemaResult { schema: vals }
+impl TryFrom<SchemaAsIpc<'_>> for SchemaResult {
+ type Error = ArrowError;
+
+ fn try_from(schema_ipc: SchemaAsIpc) -> ArrowResult<Self> {
Review Comment:
My point is twofold:
* We clearly are missing a test as the change you have made to the protocol
should be failing a test somewhere
* We need to update the logic to interpret the `SchemaResult` which does
exist and following this PR will be incorrect
_As an aside I do really wonder why arrow flight is using opaque byte
objects, the whole point of using an API DSL is to avoid this but hey ho, this
protocol is wild_
--
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]
