-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update Rust crate ureq to v3 #15759
Update Rust crate ureq to v3 #15759
Conversation
e92a4d6
to
6fde901
Compare
CodSpeed Performance ReportMerging #15759 will improve performances by 4.59%Comparing Summary
Benchmarks breakdown
|
|
crates/ruff_benchmark/src/lib.rs
Outdated
|
||
let content = response.into_string()?; | ||
let content = String::from_utf8_lossy(&response.body_mut().read_to_vec()?).to_string(); |
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.
could this just be
let content = String::from_utf8_lossy(&response.body_mut().read_to_vec()?).to_string(); | |
let content = response.body_mut().read_to_string()?; |
? Or are the semantics of String::from_utf8_lossy()
important here?
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 haven't really debugged why but it throws an error:
thread 'main' panicked at crates/ruff_benchmark/benches/formatter.rs:45:42:
called `Result::unwrap()` on an `Err` value: Request(Io(Error { kind: InvalidData, message: "stream did not contain valid UTF-8" }))
which seems unexpected given that read_to_string
should convert incorrect UTF-8 chars to ?
character.
Cargo.lock
Outdated
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.
seems like quite a few new dependencies :/ do you know if any might be avoidable?
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 don't think so, it now switched to the http
crate instead of implementing it's own Response
and Request
struct (https://github.com/algesten/ureq/blob/main/MIGRATE-2-to-3.md) plus added some more dependencies
Not sure what's causing the perf regression, will look into it later. |
7a73b99
to
b73e710
Compare
|
||
assert!( | ||
lexer.finish().is_empty(), | ||
"Input to be valid python source code" |
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.
"Input to be valid python source code" | |
"Input should be valid python source code" |
@@ -121,7 +121,13 @@ impl TestFile { | |||
// File not yet cached, download and cache it in the target directory | |||
let response = ureq::get(url.as_str()).call()?; | |||
|
|||
let content = response.into_string()?; | |||
// Using `with_config` here because without, the `loosy_utf8` results in invalid |
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.
// Using `with_config` here because without, the `loosy_utf8` results in invalid | |
// Using `with_config` here because without, the `lossy_utf8` results in invalid |
I plan to remove the entire download functionality and instead commit the files. It should simplify the benchmarks, reduce our dependencies and allow running benchmarks even when offline |
See #15878 |
Renovate Ignore NotificationBecause you closed this PR without merging, Renovate will ignore this update. You will not get PRs for any future If you accidentally closed this PR, or if you changed your mind: rename this PR to get a fresh replacement PR. |
This PR contains the following updates:
2.9.6
->3.0.0
Warning
Some dependencies could not be looked up. Check the Dependency Dashboard for more information.
Release Notes
algesten/ureq (ureq)
v3.0.1
Compare Source
v3.0.0
Compare Source
url
crate (#943)Config::save_redirect_history
(#939)Configuration
📅 Schedule: Branch creation - "before 4am on Monday" (UTC), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about this update again.
This PR was generated by Mend Renovate. View the repository job log.