-
Notifications
You must be signed in to change notification settings - Fork 25
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
container: Drop async_compression + support zstd:chunked #622
Conversation
This is basically just a workaround for Nullus157/async-compression#271 However, in practice I think we may as well just use a native blocking tokio thread here. There's a lot of shenanigans going on though because we're wrapping sync I/O with async and then back to sync because the tar code we're using is still sync. What would be a lot better is to move the compression to be inline with the sync tar parsing, but that would require some API changes and more code motion.
83d9521
to
62c4564
Compare
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.
I'm not knowledgeable enough in Rust to fully understand this but the tests passing are a good look here. LGTM
I'll test that e2e with rpm-ostree. |
I've pushed
I get a "mixed" gzip/zstd-chunked image:
And this gives me:
|
Hum, OK this is weird, I've re-tried it and it worked! So looking good. Looks like it was only a quay.io infra flake. So this LGTM. |
This is basically just a workaround for Nullus157/async-compression#271
However, in practice I think we may as well just use
a native blocking tokio thread here.
There's a lot of shenanigans going on though because
we're wrapping sync I/O with async and then back to sync
because the tar code we're using is still sync.
What would be a lot better is to move the compression to be
inline with the sync tar parsing, but that would require some
API changes and more code motion.
Closes: #608