Skip to content

Chore: fix emulator tests due to changes in reqwest#401

Merged
mbrobbel merged 1 commit into
apache:mainfrom
alamb:alamb/fix_emulator_tests
Jun 6, 2025
Merged

Chore: fix emulator tests due to changes in reqwest#401
mbrobbel merged 1 commit into
apache:mainfrom
alamb:alamb/fix_emulator_tests

Conversation

@alamb

@alamb alamb commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

The CI tests started failing on main

I tracked the issue down to starting with the releas of reqwest 0.12.16: https://crates.io/crates/reqwest/0.12.16

See the changelog here: https://github.com/seanmonstar/reqwest/releases/tag/v0.12.16

What changes are included in this PR?

  1. Change proxy url to something that is actually invalid
  2. Update retry loop count (I will comment inline on the upstream changes that required each change)

Are there any user-facing changes?

Comment thread src/client/retry.rs

// Handles redirect loop
for _ in 0..10 {
for _ in 0..11 {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe this change is needed due to seanmonstar/reqwest#2664

without this change the test fails because the redirect loop fails to trigger the limit

Comment thread src/aws/builder.rs
.with_bucket_name("bucket_name")
.with_allow_http(true)
.with_proxy_url("asdf://example.com")
// use invalid url

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I suspect, but don't know, that this change is required by

the test expects the proxy config to fail, but after the reqwest upgrade it was passing

Comment thread src/gcp/builder.rs
.with_service_account_path(service_account_path.to_str().unwrap())
.with_bucket_name("foo")
.with_proxy_url("asdf://example.com")
// use invalid url

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as above

@mbrobbel mbrobbel merged commit 7d35acf into apache:main Jun 6, 2025
8 checks passed
@alamb alamb deleted the alamb/fix_emulator_tests branch June 6, 2025 09:54
@alamb

alamb commented Jun 6, 2025

Copy link
Copy Markdown
Contributor Author

Thank you @tustvold and @mbrobbel

AdamGS pushed a commit to AdamGS/arrow-rs-object-store that referenced this pull request Sep 4, 2025
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.

Emulator tests are broken on main

3 participants