-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Upgrade dependencies, including http and hyper, where possible. #233
Conversation
Breaks docs. Signed-off-by: Omar Zabala-Ferrera <73452461+ozabalaferrera@users.noreply.github.com>
@jcrossley3 or @Lazzaretti would you be able to review this? |
I forgot about the example projects and other archs 🤦♂️. I'll work on fixing these. |
I could use some help with the failing tests. I'm not sure what is causing this error: It happens when running the test: cargo test --target wasm32-wasi --features "http-0-2-binding hyper-0-14 hyper_wasi" But the below works, so I imagine some testing dependency is the issue. cargo build --target wasm32-wasi --features "http-0-2-binding hyper-0-14 hyper_wasi" I'm not really sure if that is indicative of a real error, especially since building the example works: cd example-projects/wasi-example/
cargo build --target wasm32-wasi That being said, running causes an error, but I think it is due to my environment. cd example-projects/wasi-example/
cargo run --target wasm32-wasi
I've not worked with wasm, so any help would be appreciated. |
@ozabalaferrera the problem seems to be that the updated Mockito crate pulls in tokio (which the original version did not). I am able to build and test for the wasm32-wasi target when I use Cargot.toml ...
mockito = "0.31.1"
... I have then installed wasmedge (https://wasmedge.org/) and then run the tests successfully using
|
403bffb
to
bf18f7c
Compare
Thanks, @sophokles73 ! I've been pretty busy, but took a minute to apply your fix. It looks like all tests are passing now. |
@jcrossley3 @Lazzaretti any chance you'll find the time to take a look? |
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.
This is a lot of work, thanks!
unsafe { | ||
std::env::set_var("RUST_LOG", "axum_example=debug,tower_http=debug") | ||
} |
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.
Why is the unsafe
needed here? Just curious.
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 vaguely remember it was needed because set_var is unsafe now. But I removed it and it seems fine. Not sure what happened before 🤷 ... I pushed a commit removing the two uses of unsafe.
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 forgot to sign off the commit, so I had to overwrite that commit history to fix it.
Let me know if you'd like me to squash or clean things up in any other way.
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.
The pipeline still has a problem because of the signoff.
c195938
to
1718174
Compare
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.
Thanks a lot for the PR, and sorry for the late review!
delegate-attr, base64, snafu, bitflags, hostname, and serde_yaml. Signed-off-by: Omar Zabala-Ferrera <73452461+ozabalaferrera@users.noreply.github.com>
d07a024
to
e8f71dc
Compare
@sophokles73 do you happen to know what this is about:
I didn't even know EDIT: Ah, looks like we maybe hit the I changed the target in the workflow. This should be ready for another test run, @Lazzaretti or @jcrossley3. Sorry for the back and forth! |
Signed-off-by: Omar Zabala-Ferrera <73452461+ozabalaferrera@users.noreply.github.com>
@jcrossley3 , are you able to reach @Lazzaretti and see if we can get this PR reviewed and merged? I'd like to help with other issues after this. |
Sorry @ozabalaferrera, approved :) |
@Lazzaretti, no worries, thanks for the merge! What is the process for cutting and publishing a release to crates.io? Any estimate on when that might be? EDIT: |
Hi @ozabalaferrera , if we can fix the issue in the main branch and have a green pipeline, I think we will soon be ready to release :) |
@Lazzaretti , I opened a PR to address that issue here: #248 |
Description
This PR is the result of the discussion in #227. It upgrades various dependencies, including http and hyper, to the latest versions.
Status
This PR is ready for review. I worked on this sporadically, so I am sure there are better ways to accomplish some of what I did here.
Excluded
warp and actix have not been upgraded to http v1; I found it difficult to convert between types or implement traits. Therefore, those bindings here are mostly unchanged. I also created a separate feature flag for the old http v0.2.
Testing
I ran all the commands on the contributing guide. I also ran
cargo test --all --features
with all the features individually.