-
Notifications
You must be signed in to change notification settings - Fork 34
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
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…com/iotaledger/iota into fix/update-async-graphql-axum-tonic
async-graphql
, axum
and tonic
…com/iotaledger/iota into fix/update-async-graphql-axum-tonic
async-graphql
, axum
and tonic
async-graphql
, axum
and tonic
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.
lgtm 🦀
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.
blocking to assess compatibility with released versions of the indexer
There is a compatibility check that ensures that all relations in |
@@ -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>, |
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 did we change that one?
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.
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)
Closes #6886
In order to use the latest
async-graphql
,axum
andtonic
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 asasync-graphql-axum
has moved toaxum
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@77c3334I removed the
{
->:
path patcher as{}
is the correct syntax inaxum
>0.8
I don't think they were going to release a patch over the
7.0.11
release ofasync-graphql-axum
either btw, I mean, considering they updated toaxum
0.8
somewhere in between7.0.11
and7.0.16
I think it's safe to safe this is anti-semver territoryI didn't want to fork the
7.0.11
version and backport the patch, so I went on updating the other depsMaybe 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]