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

Consider improving Reactor chain and context propagation #346

Closed
ciscoo opened this issue Apr 1, 2022 · 2 comments
Closed

Consider improving Reactor chain and context propagation #346

ciscoo opened this issue Apr 1, 2022 · 2 comments
Assignees

Comments

@ciscoo
Copy link

ciscoo commented Apr 1, 2022

From #328 (comment):

(Emphasis mine)

Currently we decorate each DataFetcher and propagate to it Reactor context from the web layer. Apart from that they are not in a single Reactor chain and Reactor context from one cannot be seen by others. This is something we might try to improve in the future, but for now if you want pass something across data fetchers, just use GraphQLContext.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 1, 2022
@bclozel
Copy link
Member

bclozel commented Sep 8, 2022

I think that #459 might have solved this, as the chain of contexts (threadlocal, reactor context, graphql conetxt) is now consistent for the entire execution.

@bclozel bclozel added the for: team-attention An issue we need to discuss as a team to make progress label Sep 8, 2022
@bclozel bclozel removed the for: team-attention An issue we need to discuss as a team to make progress label Sep 22, 2022
@rstoyanchev
Copy link
Contributor

We do have improved context propagation after #459. However, Reactor DataFetchers are still not in a single chain of execution given that graphql.execution.AsyncExecutionStrategy expects each to return CompletableFuture and joins them as such for the final result. So the main way to pass context from one DataFetchers to others remains via GraphQLContext, which should be fine to use. I'm going to close this for now as there isn't anything further we intend to do here at this time.

@rstoyanchev rstoyanchev closed this as not planned Won't fix, can't repro, duplicate, stale Sep 22, 2022
@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged label Sep 22, 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

No branches or pull requests

4 participants