Update network (grid download) functionality to use ureq 3.x #228
Conversation
urschrei
force-pushed
the
ureq-3-update
branch
2 times, most recently
from
9de3ebc to
517fe65
Compare
|
I don't know why tests are only failing with that dependency mismatch on 1.82, but incorporating #229 causes this PR to compile for me locally with |
urschrei
force-pushed
the
ureq-3-update
branch
2 times, most recently
from
37d4fd8 to
a309938
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR updates the network (grid download) functionality to work with ureq 3.x, adjusting API calls and error handling accordingly. Key changes include:
- Updating dependency versions (ureq to 3.0.11, adding http dependency) in Cargo.toml.
- Refactoring the network code in src/network.rs to accommodate the new ureq API, including changes in error handling and header extraction.
- Minor adjustments in CHANGES.md to document the update.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/network.rs | Refactored network functions to use the new ureq 3.x API including error handling and header processing. |
| Cargo.toml | Updated dependency versions and modified the network feature list. |
| CHANGES.md | Documented the update to ureq 3.x. |
- [x] I agree to follow the project's [code of conduct](https://github.com/georust/.github/blob/main/CODE_OF_CONDUCT.md). - [x] I added an entry to the project's change log file if knowledge of this change could be valuable to users. - Usually called `CHANGES.md` or `CHANGELOG.md` - Prefix changelog entries for breaking changes with "BREAKING: " --- We're using an older flate2 version that won't be accepted by ureq 3.x (see #228).
src/network.rs
Outdated
| .headers_names() | ||
| .into_iter() | ||
| .headers() | ||
| .keys() |
There was a problem hiding this comment.
Here we're iterating over the keys and looking up the values again in the filter_map below.
There was a problem hiding this comment.
Good catches. Simplified with an iterator now.
src/network.rs
Outdated
| .headers_names() | ||
| .into_iter() | ||
| .headers() | ||
| .keys() |
src/network.rs
Outdated
| // Start retrying: up to MAX_RETRIES | ||
| while (SERVER_ERROR_CODES.contains(&res.status()) || RETRY_CODES.contains(&res.status())) | ||
| while (SERVER_ERROR_CODES.contains(&res.status().as_u16()) | ||
| || RETRY_CODES.contains(&res.status().as_u16())) | ||
| && retries <= MAX_RETRIES | ||
| { | ||
| retries += 1; | ||
| let wait = time::Duration::from_millis(get_wait_time_exp(retries as i32)); |
There was a problem hiding this comment.
Unrelated, but exponential back-off (a very popular approach) picks a random time between 0 and an exponentially-increasing limit.
There was a problem hiding this comment.
Ha fair. Rather than "fix" it, I've renamed it to _quad and updated the docstring.
There was a problem hiding this comment.
Oh wow, I didn't even notice it's retries^2, not 2^retries.
CHANGES.mdorCHANGELOG.mdNo changelog entry yet since we have other PRs in flight.This was an unfun piece of work as the ureq API has evolved significantly between 2.x and 3.0, and the way we're passing the connection pool object around is more difficult to do now. That having been said the tests pass and none of the changes affect the public API. We also have a new optional dep in the form of
http, but I suspect it was present previously anyway.