-
Notifications
You must be signed in to change notification settings - Fork 160
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 CarWriter sink #3461
Add CarWriter sink #3461
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. We need to delete all references to fvm_ipld_car
but that can be done in a follow-up PR.
I found the (one?) source of the non-determinism. Looks like if I remove the call to This is still puzzling me because the previous file created using However I find that this file doesn't deflate well:
This is not the case for the new one, and I have same size and checksum once deflated (compared to the old method but without compression):
|
Co-authored-by: David Himmelstrup <[email protected]>
Could you write a QuickCheck test that passes data through CarWriter and CarStream, making sure the data isn't changed? |
I've did some experiments and managed to match the But then trying to deflate or |
I'm not sure about this implementation approach - I'll get back to you in a moment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you have a look at this, and then we'll chat
I think the approach is fine. Offloading frame encoding to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's refactor this once Nullus157/async-compression#246 is fixed
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #3192
Other information and links
Change checklist