Skip to content

Conversation

@maxburke
Copy link
Contributor

In the previous commit I had the continuation marker check in the wrong location.

@github-actions
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@maxburke maxburke changed the title Continuation marker check was in wrong location. ARROW-8225: [Rust] Continuation marker check was in wrong location. Mar 31, 2020
@github-actions
Copy link

@maxburke
Copy link
Contributor Author

Hm; these errors popped up but I'm not sure why they're happening?

32 | |
33 | | #[tonic::async_trait]
   | |_____________________^ missing `DoExchangeStream`, `do_exchange` in implementation
   |
   = help: implement the missing item: `type DoExchangeStream = Type;`
   = help: implement the missing item: `fn do_exchange(&'life0 self, _: tonic::request::Request<tonic::codec::decode::Streaming<flight::FlightData>>) -> std::pin::Pin<std::boxed::Box<(dyn core::future::future::Future<Output = std::result::Result<tonic::response::Response<<Self as flight::flight_service_server::FlightService>::DoExchangeStream>, tonic::status::Status>> + std::marker::Send + 'async_trait)>> { unimplemented!() }`

Is the trunk broken?

@nevi-me
Copy link
Contributor

nevi-me commented Apr 1, 2020

Hm; these errors popped up but I'm not sure why they're happening?

32 | |
33 | | #[tonic::async_trait]
   | |_____________________^ missing `DoExchangeStream`, `do_exchange` in implementation
   |
   = help: implement the missing item: `type DoExchangeStream = Type;`
   = help: implement the missing item: `fn do_exchange(&'life0 self, _: tonic::request::Request<tonic::codec::decode::Streaming<flight::FlightData>>) -> std::pin::Pin<std::boxed::Box<(dyn core::future::future::Future<Output = std::result::Result<tonic::response::Response<<Self as flight::flight_service_server::FlightService>::DoExchangeStream>, tonic::status::Status>> + std::marker::Send + 'async_trait)>> { unimplemented!() }`

Is the trunk broken?

#6800 will fix the issue

@nevi-me
Copy link
Contributor

nevi-me commented Apr 1, 2020

@maxburke do you perhaps have an example to help me understand the continuation issue? I haven't read the spec in a few weeks/months

@maxburke
Copy link
Contributor Author

maxburke commented Apr 1, 2020

@nevi-me https://arrow.apache.org/docs/format/Columnar.html

The encapsulated binary message format is as follows:

A 32-bit continuation indicator. The value 0xFFFFFFFF indicates a valid message. This component was introduced in version 0.15.0 in part to address the 8-byte alignment requirement of Flatbuffers

A 32-bit little-endian length prefix indicating the metadata size

The message metadata as using the Message type defined in Message.fbs

Padding bytes to an 8-byte boundary

The message body, whose length must be a multiple of 8 bytes

@paddyhoran
Copy link
Contributor

@nevi-me I think your original IPC implementation was based on 0.14, right before this change was adopted.

Copy link
Contributor

@nevi-me nevi-me left a comment

Choose a reason for hiding this comment

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

Thanks, I've now seen the change from 0.15.0, and understand what this change achieves

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants