Skip to content

Start axum migration #1903

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

Merged
merged 9 commits into from
Nov 19, 2022
Merged

Start axum migration #1903

merged 9 commits into from
Nov 19, 2022

Conversation

syphar
Copy link
Member

@syphar syphar commented Nov 12, 2022

This starts the migration from iron to axum (see #1900 )

We're using a strangler-fig pattern, so we're running the old iron server in the background, which will be called when we don't find a route in axum. The service is based on adapted code from axum-strangler.

I migrated some handlers from web::sitemap and some static redirects, especially to show how I thing we should do:

  • template rendering
  • CSP
  • error handling without ctry, cexpect or .unwrap/.expect
  • request metrics recorder
  • cache middleware

especially the combination of template rendering and CSP took some thinking, but IMO this is a good approach.

There is some duplication (impl_webpage + impl_axum_webpage, AxumNope + Nope, AxumErrorPage + ErrorPage), which was easier to do than figuring out how to design for both Iron & Axum in these structs. And since at some point Iron will be gone, we're fine IMO.

I tried to do most of the change in our TestFrontend so the tests just run as they are, so we're sure everything is fine.

other things that will come / are good to know

( see also master...syphar:docs.rs:new-web-axum )

  • in web::routes -> a get_rustdoc will also inject a prefix-blocking middleware
  • the old shared-resources handler will also be a middleware, if the rebuild isn't finished until then and we can replace it with a handler.
  • trailing-slash handling is done by default in the axum router
  • axum/tower graceful shutdown will be added when iron is gone

pending manual test

  • do redirects include https, coming from cloudfront? ( see redirect_base)
  • are request metrics recorded correctly?
  • sentry error reporting / trace

which specifics are not covered with tests good enough?

@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Nov 12, 2022
@syphar syphar force-pushed the start-axum branch 2 times, most recently from 929a74b to efab7d8 Compare November 12, 2022 17:30
@syphar
Copy link
Member Author

syphar commented Nov 12, 2022

cc @rust-lang/docs-rs for review, tests, inpout

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

this is great :) I left a bunch of comments but nothing looks blocking.

@syphar syphar added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Nov 14, 2022
@syphar syphar merged commit 879b8e1 into rust-lang:master Nov 19, 2022
@syphar syphar deleted the start-axum branch November 19, 2022 04:50
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Nov 19, 2022
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.

2 participants