Repository navigation
Conversation
…ock engages RelationConnection#async_dataloader? read `context[:dataloader]`, but Multiplex stores the dataloader on the multiplex context, not on the query context a connection holds, so the key is nil for schemas that `use GraphQL::Dataloader::AsyncDataloader` and the locks added in rmosolgo#5708 and rmosolgo#5717 never engaged. Read it through Query::Context#dataloader instead, which falls back to the multiplex dataloader, and build the Fiber specs' connections from a real query context so they exercise the same path execution does.
drhops
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RelationConnection#async_dataloader?reads@context[:dataloader]. In a real query, that key is never set on the query context (the dataloader lives on the multiplex), so the lookup returns nil and the locks from #5708 and #5717 never engage. UnderAsyncDataloader,edgesandpageInfostill load the page twice.The check now uses
@context&.dataloader, whichQuery::Contextresolves through the multiplex. This slipped through because both PRs' specs built the connection with a Hash context, where[:dataloader]works.The specs now use a real query context via
GraphQL::Query.new(...).context. Note a plain Hash context no longer satisfies the check.