oras-project / rust-oci-client

A Rust crate to interact with OCI registries
Apache License 2.0
86 stars 45 forks source link

feat(client): allow to set read_timeout and connect_timeout #137

Closed SuperTux88 closed 1 month ago

SuperTux88 commented 1 month ago

Allow to set read_timeout and connect_timeout on the underlying client. Otherwise the pull will just hang forever if you have an unstable connection and there was an interruption.

I wasn't sure if it would be useful to set some defaults here, but decided against it, because there was no timeout before (so no change for existing implementations) and there is also no timeout set in the reqwest client by default. And I also wasn't sure what useful defaults would be (since that maybe depends on your connection?). But let me know if you want me to set a default value for these.

SuperTux88 commented 1 month ago

So how should I handle this failed build about the licenses? I think this has nothing to do with my change. I changed the reqwest version to 0.12.4, because the read_timeout was only just added to this release (see release notes). But since there is no commited Cargo.lock, this version should already have been used before, as this release is already 2 Month old. I just wanted to make the version requirement more explicit (so in case you still have an old Cargo.lock locally, it updates the version).

But as there is no commited Cargo.lock, it also just uses the newest version of all the other things. And it looks like there was a new version of idna which got updated from 0.5.0 to 1.0.0 3 days ago, and this new version has a whole bunch of new dependencies, one of which also brings in this new license, which isn't allowed yet?

So, should I just add the license to the allowed list of licenses, or should I pin idna to 0.5.0 again, so it uses the old version? Or should I just ignore the issue about the build and you'll deal with that outside of this PR, as it's not related?

thomastaylor312 commented 1 month ago

@SuperTux88 You beat me by 6 minutes 😆. I just pushed up a fix: https://github.com/krustlet/oci-distribution/pull/138

SuperTux88 commented 1 month ago

@thomastaylor312 Rebased, build is now green :partying_face: Thanks for the fix.