Skip to content

fix(core): Update async-graphql, axum and tonic #6901

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

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

marc2332
Copy link
Contributor

@marc2332 marc2332 commented May 19, 2025

Closes #6886

In order to use the latest async-graphql, axum and tonic also need to be updated in our codebase, otherwise it would throw the classic "you might be using two versions of the same crate" error as async-graphql-axum has moved to axum 0.8 since the version we use (7.0.11)

Also had to update tonic-rustls, but this one is not released, so I pinned on bmwill/tonic-rustls@77c3334

I removed the { -> : path patcher as {} is the correct syntax in axum >0.8

image

I don't think they were going to release a patch over the 7.0.11 release of async-graphql-axum either btw, I mean, considering they updated to axum 0.8 somewhere in between 7.0.11 and 7.0.16 I think it's safe to safe this is anti-semver territory

I didn't want to fork the 7.0.11 version and backport the patch, so I went on updating the other deps

Maybe you disagree with this and would prefer with a simple fork, but this will just make it harder in the future if other issues come up

[run-ci]

Copy link

vercel bot commented May 19, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
apps-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 8:03am
apps-ui-kit ✅ Ready (Inspect) Visit Preview May 20, 2025 8:03am
rebased-explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 8:03am
wallet-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2025 8:03am

@iota-ci iota-ci added the tooling Issues related to the Tooling team label May 19, 2025
@marc2332 marc2332 changed the title fix: Update async-graphql, axum and tonic fix: Update async-graphql, axum and tonic May 19, 2025
@marc2332 marc2332 marked this pull request as ready for review May 19, 2025 16:06
@marc2332 marc2332 requested review from a team as code owners May 19, 2025 16:06
@marc2332 marc2332 changed the title fix: Update async-graphql, axum and tonic fix(core): Update async-graphql, axum and tonic May 20, 2025
Copy link
Contributor

@kodemartin kodemartin left a comment

Choose a reason for hiding this comment

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

lgtm 🦀

Copy link
Contributor

@kodemartin kodemartin left a comment

Choose a reason for hiding this comment

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

blocking to assess compatibility with released versions of the indexer

@kodemartin
Copy link
Contributor

blocking to assess compatibility with released versions of the indexer

There is a compatibility check that ensures that all relations in iota_indexer::schema exist in the database the graphql service tries to connect to. Should be fine as the two services are shipped together.

@@ -68,7 +68,7 @@ impl ApiEndpoint<RestService> for ExecuteTransaction {
async fn execute_transaction(
State(state): State<Option<Arc<dyn TransactionExecutor>>>,
Query(parameters): Query<ExecuteTransactionQueryParameters>,
client_address: Option<axum::extract::ConnectInfo<SocketAddr>>,
client_address: Result<axum::extract::ConnectInfo<SocketAddr>, ExtensionRejection>,
Copy link
Contributor

Choose a reason for hiding this comment

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

why did we change that one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its an axum change, it does no longer take an Option, but instead it takes a Result now. Took me some time to figure out why, ended up in tokio-rs/axum#3245 (comment)

@lzpap lzpap added this to the v1.2.x - protocol v8 milestone May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Issues related to the Tooling team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[graphql-rpc]: graphiql does not work
8 participants