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.

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

Replace rouille with axum #2234

Open
Xuanwo opened this issue Jul 25, 2024 · 5 comments
Open

Replace rouille with axum #2234

Xuanwo opened this issue Jul 25, 2024 · 5 comments

Comments

@Xuanwo
Copy link
Collaborator

Xuanwo commented Jul 25, 2024

Since we are heavily using tokio and reqwest, it would be more sensible to use an asynchronous web framework rather than a blocking one.

Migrating to axum will have the following benefits too:

  • It's easier to maintain the API than the macro.
  • Remove some old future-incompatibile deps:
warning: the following packages contain code that will be rejected by a future version of Rust: buf_redux v0.8.4, multipart v0.18.0
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`
@Saruniks
Copy link
Contributor

Saruniks commented Jul 25, 2024

Have you considered porting rouille -> warp changes from cachepot? (Port remaining cachepot changesets)

(cachepot rouille -> warp merge request)

@Xuanwo
Copy link
Collaborator Author

Xuanwo commented Jul 25, 2024

Wow, that's great work, respect!

I don't have a strong opinion on Warp and Axum as long as all tests pass. I believe that paritytech/cachepot#131 has completed the most challenging part of the migration work. This will significantly simplify our tasks, especially if we decide to use axum in the future.

I's happy to review and get it merged if someone port this PR here.


I will get it a try first to see how many conflicts there:

oh...

Auto-merging .github/workflows/ci.yml
CONFLICT (content): Merge conflict in .github/workflows/ci.yml
Auto-merging Cargo.lock
CONFLICT (content): Merge conflict in Cargo.lock
Auto-merging Cargo.toml
...

@Saruniks
Copy link
Contributor

Saruniks commented Jul 25, 2024

Indeed. More merge conflicts than expected.

rouille -> warp is not my work, but if you would like to and if it's not time critical I could try solving merge conflicts and testing.

@Xuanwo
Copy link
Collaborator Author

Xuanwo commented Jul 25, 2024

rouille -> warp is not my work, but if you would like to and if it's not time critical I could try solving merge conflicts and testing.

Thanks a lot for picking up this work!

@Saruniks
Copy link
Contributor

I'm in the progress with this task. I'm troubleshooting flakiness that I get while testing after merging the changes.

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

No branches or pull requests

2 participants