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

feat: Return number of bytes written for every operation #11

Merged

Conversation

matheus23
Copy link
Contributor

The main change is to return usize from CarWriter::write and CarWriter::write_header. These usizes indicate the amount of bytes written to the underlying AsyncWrite.

Knowing these numbers is useful to us (at fission), since it allows us to package up car files "until at least X bytes were written into the CAR file".

I added a proptest to make sure that the final CAR file is just as big as all the returned usizes summed up. (I initially messed up the impl. proptest makes stuff like this easy to fix though.)

Copy link
Contributor

@dignifiedquire dignifiedquire left a comment

Choose a reason for hiding this comment

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

nice

@dignifiedquire
Copy link
Contributor

sigh wasm builds are broken

@matheus23
Copy link
Contributor Author

sigh wasm builds are broken

Is that on me? cargo build --target wasm32-unknown-unknown works for me locally.

I saw there's some failures due to 1.63 being the MSRV, not 1.60 anymore. I pushed a commit that pushes the MSRV to 1.63.

@dignifiedquire
Copy link
Contributor

wasn’t you, lets see if your fix helps

@matheus23
Copy link
Contributor Author

Ah I vaguely remembered seeing something related recently in rs-ucan. Followed the traces back and found this:
subconsciousnetwork/noosphere#514

Similar changes will probably fix this here, too. Let me commit them real quick.

@matheus23
Copy link
Contributor Author

I think this should be ready to merge ✌️

@matheus23
Copy link
Contributor Author

I ran CI on my fork, it's running through fine there. I swear I'm done done for good now.

@matheus23 matheus23 mentioned this pull request Sep 1, 2023
@dignifiedquire dignifiedquire merged commit f44f4af into n0-computer:main Sep 1, 2023
@matheus23
Copy link
Contributor Author

🎉

@dignifiedquire
Copy link
Contributor

published in 0.4.0

@matheus23 matheus23 deleted the matheus23/return-bytes-written branch February 22, 2024 18:05
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.

2 participants