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

Fixing hasNext behaviour #1745

Merged
merged 14 commits into from
Sep 14, 2022
Merged

Fixing hasNext behaviour #1745

merged 14 commits into from
Sep 14, 2022

Commits on Sep 12, 2022

  1. Revert "Set correctly hasNext for the last chunk of a deferred resp…

    …onse (#1736)"
    
    This reverts commit 24a00e6.
    Geal committed Sep 12, 2022
    Configuration menu
    Copy the full SHA
    88f24e5 View commit details
    Browse the repository at this point in the history
  2. WIP: attempt at fixing hasNext

    we previously returned an empty graphql response at the end of the
    response stream to set the `hasNext` field to false, to indicate that no
    more responses will come.
    
    That empty response is causing issues in some clients, so 24a00e6 was a
    fix to set the `hasNext` on a deferred response from inside query
    planner execution, but it does not account for parallel deferred
    response executions, so one response might come with `hasNext` to false
    then get another one.
    
    This commit attempts another solution, where we go through an
    intermediate task that checks if the response stream is closed (it is a
    channel, so it implements `FusedStream`). Unfortuantely, right now it
    fals to recognize when the stream is closed
    Geal committed Sep 12, 2022
    Configuration menu
    Copy the full SHA
    8d8ae23 View commit details
    Browse the repository at this point in the history
  3. properly detect the end of stream

    since the stream is marked as terminated from inside poll_next, we need
    to call it a second time after getting a message, to check if it is
    closed, but we cannot do that with an async method, since we need to
    send the current message ASAP. So we call `try_next`, and depending on
    its result, we either send the current message, or set hasNext on the
    current message and send it if the channel is closed, or send the
    current message, get the next one and try again to see if there's
    another one
    Geal committed Sep 12, 2022
    Configuration menu
    Copy the full SHA
    69ab5bd View commit details
    Browse the repository at this point in the history
  4. cleanup

    Geal committed Sep 12, 2022
    Configuration menu
    Copy the full SHA
    1d83d78 View commit details
    Browse the repository at this point in the history

Commits on Sep 13, 2022

  1. Configuration menu
    Copy the full SHA
    be8a0ab View commit details
    Browse the repository at this point in the history
  2. remove println

    Geal committed Sep 13, 2022
    Configuration menu
    Copy the full SHA
    0093897 View commit details
    Browse the repository at this point in the history
  3. try to fix

    Geal committed Sep 13, 2022
    Configuration menu
    Copy the full SHA
    227e0cb View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    31b2735 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    1efd9a4 View commit details
    Browse the repository at this point in the history
  6. filter empty responses

    Geal committed Sep 13, 2022
    Configuration menu
    Copy the full SHA
    17e383a View commit details
    Browse the repository at this point in the history
  7. fix

    Geal committed Sep 13, 2022
    Configuration menu
    Copy the full SHA
    fda59d3 View commit details
    Browse the repository at this point in the history

Commits on Sep 14, 2022

  1. detect if the stream disconnected before marking the last message

    there could be a race condition where we consume and send all messages,
    then await for the next one, then the stream is closed and we don't have
    any message on which we would set `has_next`. So we detect that case and
    add a last message
    Geal committed Sep 14, 2022
    Configuration menu
    Copy the full SHA
    5210765 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    2ddb5e9 View commit details
    Browse the repository at this point in the history
  3. changelog

    Geal committed Sep 14, 2022
    Configuration menu
    Copy the full SHA
    d91fa00 View commit details
    Browse the repository at this point in the history