Skip to content
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.

Sign up for GitHub

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

Jump to bottom

azure_core 0.19.0 fails to build for wasm when using reqwest #1554

Closed
johnbatty opened this issue Jan 7, 2024 · 0 comments · Fixed by #1555
Closed

azure_core 0.19.0 fails to build for wasm when using reqwest #1554

johnbatty opened this issue Jan 7, 2024 · 0 comments · Fixed by #1555

Comments

@johnbatty
Copy link
Contributor

A recent fix #1550 added a call to reqwest::ClientBuilder::pool_max_idle_per_host(). Unfortunately this method does not exist in the reqwest wasm implementation:

error[E0599]: no method named `pool_max_idle_per_host` found for struct `ClientBuilder` in the current scope
  --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/azure_core-0.19.0/src/http_client/reqwest.rs:18:10
   |
17 |       let client = ::reqwest::ClientBuilder::new()
   |  __________________-
18 | |         .pool_max_idle_per_host(0)
   | |         -^^^^^^^^^^^^^^^^^^^^^^ method not found in `ClientBuilder`
   | |_________|
   |

I noticed this when migrating the azure-devops-rust-api to 0.19.0 (the CI build broke). Not sure why the azure-sdk-for-rust CI build didn't fail - I believe it also does a wasm build. I guess it may not enable the enable_reqwest feature?

The fix is obviously to only include the new code for non-wasm builds.
@johnbatty johnbatty mentioned this issue Jan 7, 2024
Merged
demoray pushed a commit to demoray/azure-sdk-for-rust that referenced this issue Jan 8, 2024
check wasm with default features
667d80e 
As indicated in Azure#1554, we're not catching changes that impact real-world
WASM use in CICD.

Originally, our WASM implementation was a best-effort path as reqwest
(our first-class supported HTTP backend) did not support WASM.  Now that
reqwest supports WASM, we should update our CICD to validate WASM with
the default features.
@demoray demoray mentioned this issue Jan 8, 2024
Merged
demoray added a commit that referenced this issue Jan 8, 2024
@demoray
check wasm with default features (#1557)
aff7692 
As indicated in #1554, we're not catching changes that impact real-world
WASM use in CICD.

Originally, our WASM implementation was a best-effort path as reqwest
(our first-class supported HTTP backend) did not support WASM.  Now that
reqwest supports WASM, we should update our CICD to validate WASM with
the default features.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove call to reqwest::ClientBuilder::pool_max_idle_per_host() on wasm johnbatty/azure-sdk-for-rust
1 participant
@johnbatty