Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add type checking #14

Closed
wants to merge 4 commits into from
Closed

add type checking #14

wants to merge 4 commits into from

Conversation

pmarks
Copy link
Contributor

@pmarks pmarks commented Sep 3, 2019

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 94.866% when pulling b3fb41a on pmarks/check-types into 2f7bf36 on master.

//! Shardio storage is intended for transient use within a single processing task, not for archival purposes.
//! You can't read shardio files created by binaries for a different Rust version,
//! or where type definitions have changed. It recommended
//! to only use interact with a shardio file from a single Rust binary.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am a bit concerned about this because if I have a unit test that takes in a shard file as input (let's say this is a unit test for a stage code), won't that break when we upgrade the toolchain? Or should we not make such unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah good point. I thought that bincode also could change formats between versions, but it looks like bincode should be forward compatible with new Rust versions and new bincode versions as long as the definition of you type doesn't change. When you have a type mismatch, the most common thing is to get a 'failed to fill buffer' error or a crazy big allocation that panics without a good stack trace. So maybe using https://docs.rs/bincode/1.1.4/bincode/struct.Config.html#method.limit & setting the limit to a reasonable value, and returning a better error message if the limit is hit is a good compromise.

@pmarks pmarks closed this Sep 4, 2019
@adam-azarchs adam-azarchs deleted the pmarks/check-types branch November 8, 2021 21:09
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.

None yet

3 participants