tustvold commented on code in PR #2586:
URL: https://github.com/apache/arrow-rs/pull/2586#discussion_r955084808


##########
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:
   > I think this is just simple issue about the protocal of getSchema RPC.
   
   The problem is the server used to do one thing, and with this PR the server 
now does a different thing without updating the Rust client to understand it. 
   
   To avoid breaking existing users of Rust arrow-flight we would ideally 
update the client to support both the old and new `SchemaResult`. Otherwise 
people who have services using the Rust client with the Rust server will be 
unable to update, as there is no backwards compatibility, nor even support for 
the new encoding.
   
   Regardless of if the old encoding was incorrect, we shouldn't just break 
compatibility without at the very least proving an upgrade path, and ideally 
one that doesn't require simultaneously upgrading all servers and clients.
   
   > I have the same confusion as you, can you help to file an issue to the de 
mail list to discuss about this?
   
   I think the ship has sailed at this point, I was just expressing frustration 
at some of the protocol decisions.



-- 
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]

Reply via email to