Skip to content

Conversation

@jbms
Copy link
Contributor

@jbms jbms commented May 9, 2023

No description provided.

@jbms
Copy link
Contributor Author

jbms commented May 9, 2023

I also clarified the language a bit to account for the possibility of nested sharding.

@jbms jbms requested a review from jstriebel May 9, 2023 04:25
@jbms
Copy link
Contributor Author

jbms commented May 9, 2023

@normanz Your input would be much appreciated.

Copy link
Member

@normanrz normanrz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@clbarnes
Copy link
Contributor

clbarnes commented May 18, 2023

Worth combining this with a spec for a CRC-32C bytes->bytes codec to apply to chunks too? Seems like low-hanging fruit if we're pulling in that functionality anyway. I know this PR just deals with the chunk address footer, and they should stay separate as adding a checksum would break partial reads.

@jbms
Copy link
Contributor Author

jbms commented May 19, 2023

I agree it makes sense to add a crc32c bytes->bytes codec, but I would just assume address that in a separate PR.

Copy link
Member

@jstriebel jstriebel left a comment

Choose a reason for hiding this comment

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

LGTM, thanks 👍

@jstriebel jstriebel merged commit 46ed6cd into zarr-developers:main Jun 8, 2023
@jbms jbms deleted the sharding-checksum branch June 8, 2023 19:02
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.

4 participants