Skip to content

defer stream support within stitching#1941

Open
yaacovCR wants to merge 49 commits intomasterfrom
defer-stream
Open

defer stream support within stitching#1941
yaacovCR wants to merge 49 commits intomasterfrom
defer-stream

Conversation

@yaacovCR
Copy link
Copy Markdown
Collaborator

@yaacovCR yaacovCR commented Aug 25, 2020

A work in progress...

Help wanted...

  • test single proxied deferred patch.
  • test merging deferred patches from multiple subschemas
  • handle stream

In terms of mechanism, have settled on external values that are from asyncIterables being converted to an initial result with an associated Receiver object, which is passed down the execution tree. If a field is not present on the result, and a Receiver can be found, the default merging resolver will request to be notified when the field is available.

  • think more broadly about how to implement the Receiver, maybe we should use Repeaters rather than raw async iterables, and what the best pattern is for consuming async iterables.... see Not that great for implementing transducers? repeaterjs/repeater#48 (comment)

  • update all transforms that touch results - some result transformers parse the request and transform the response based on saved state -- these transformers may require rewriting patch labels so that state does not overlap between different fragments...

    • CheckResultAndHandleErrors (merging errors into results) - replaced by externalValueFromResult and externalValueFromPatchResult functions
    • TransformQuery
    • MapLeafValues
    • RenameRootTypes
    • RenameTypes
    • TransformCompositeFields
    • WrapFields
    • HoistFields
  • update batch-execute

  • update batch-delegate

  • test errors

  • test timeouts

graphql/graphql-js#2319
graphql/graphql-spec#742

@yaacovCR yaacovCR changed the base branch from v7 to master August 26, 2020 20:57
@yaacovCR yaacovCR closed this Aug 26, 2020
@yaacovCR yaacovCR reopened this Aug 26, 2020
@yaacovCR yaacovCR changed the base branch from master to v7 August 26, 2020 21:34
@yaacovCR yaacovCR closed this Aug 26, 2020
@yaacovCR yaacovCR reopened this Aug 26, 2020
@yaacovCR yaacovCR closed this Aug 26, 2020
@yaacovCR yaacovCR reopened this Aug 26, 2020
@yaacovCR yaacovCR changed the title enable defer stream across schema creation defer stream support Aug 27, 2020
@yaacovCR yaacovCR force-pushed the v7 branch 4 times, most recently from 1e7391d to 9269c2c Compare September 2, 2020 19:19
@yaacovCR yaacovCR force-pushed the v7 branch 5 times, most recently from bb7bd64 to 431ac4e Compare September 21, 2020 15:14
@yaacovCR yaacovCR force-pushed the v7 branch 4 times, most recently from f8134f0 to e49c793 Compare October 6, 2020 18:01
Base automatically changed from v7 to master October 23, 2020 05:09
yaacovCR and others added 6 commits May 30, 2021 00:02
yaacovCR added 4 commits June 1, 2021 10:33
We may end up having one type of Receiver
Receivers can also return external values not just MergedExecutionResults, even if the cache is of MergedExecutionsResults
It can be passed a stream of transformedResults using mapAsyncIterator

note that using `break` in for await(...of...) causes abrupt completion and will return the iterator, not really sure why this became a problem only when mapping the iterator
...to use DataLoader for the actual GraphQL results, rather than the external values

This allows us to properly transduce streams.

TODO:
1. We should now be able to add the stream directive for batchDelegateToSchema.
2. Refactor error parsing, now we are relying on the onLocatedError option, but this
   is no longer necessary. Note that additional tests for batched errors are now passing
   (see #2951)
3. Bring back createBatchDelegateFn, that may be useful
4. The Receiver is now created separately for each list item, which means that the
   initialPathDepth probably does not match? This will require a separate argument to
   the Receiver constructor for adjustment
5. Bring back valuesFromResults and fix documentation, we now operate on GraphQL
   results, so should be fewer pitfalls (see #2829)
yaacovCR added 2 commits June 1, 2021 23:39
try to avoid breaking changes (in the future?)
@sshevlyagin
Copy link
Copy Markdown

So... is this done? #4796 :D

@yaacovCR
Copy link
Copy Markdown
Collaborator Author

Not yet :( still can't stitch

@maxbol
Copy link
Copy Markdown
Contributor

maxbol commented Jan 5, 2023

Hi @yaacovCR ! Do you know if this will solve other async/persistent connections, like @n1ru4l 's live query pattern, in stitched schemas?

I would love to assist with this goal in any way possible!

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.

6 participants