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 old actix-web with axum #302

Merged
merged 3 commits into from
Apr 13, 2023
Merged

Replace old actix-web with axum #302

merged 3 commits into from
Apr 13, 2023

Conversation

Theodus
Copy link
Member

@Theodus Theodus commented Apr 12, 2023

Move away from the outdated actix-web version we've been stuck on since the Rust rewrite. Use axum instead.

Closes #300

Theodus added 2 commits April 12, 2023 14:21
This tweaks the Agora context type we use in order to avoid a recurring
Rust compiler bug (rust-lang/rust#71723) which
makes it suddenly impossible to carry the context across an await. This
comes at the cost of increasing String allocations. Though the use of
snmalloc mitigates this cost somewhat.
@Theodus Theodus requested a review from cmwhited April 12, 2023 18:51
@Theodus Theodus changed the title Theodus/axum Replace old actix-web with axum Apr 12, 2023
Copy link
Contributor

@cmwhited cmwhited left a comment

Choose a reason for hiding this comment

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

Hooray!!!! Glad we were able to get this done! 🔥 🔥

CorsLayer::new()
.allow_origin(cors::Any)
.allow_headers(cors::Any)
.allow_methods([http::Method::POST, http::Method::OPTIONS]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the gateway serve the graphql schema inspection for a subgraph? If so, do we also need to allow GET here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I only see us handling POST in the route handlers. But want to validate.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope the gateway doesn't do schema inspection, and previsously only allowed POST & OPTIONS. I just tested this out without setting OPTIONS here, and it seems to work as expected. Fixed

@Theodus Theodus merged commit 65c063c into main Apr 13, 2023
@Theodus Theodus deleted the theodus/axum branch April 13, 2023 14:31
@Theodus Theodus mentioned this pull request Apr 14, 2023
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.

Replace old actix-web with axum
2 participants