Don't delete response collectors in a transaction #369
Conversation
This file contains 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
alexjpwalker
commented
Jan 27, 2022
| @@ -47,7 +47,7 @@ private ErrorMessage(String codePrefix, int codeNumber, String messagePrefix, St | |||
| public static final Client MISSING_RESPONSE = | |||
There was a problem hiding this comment.
See comments on https://github.com/vaticle/typedb-client-python/pull/250/files, which this PR is a copy of.
alexjpwalker
commented
Jan 27, 2022
| @@ -518,6 +521,23 @@ public void testSimpleExplanation() { | |||
| }, READ, TypeDBOptions.core().infer(true).explain(true)); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testStreaming() { | |||
There was a problem hiding this comment.
With these options (the default in TypeDB at time of writing), the server may respond with: 50 answers -> CONTINUE -> 1 answer [compensating for latency] -> DONE. The client will respond to CONTINUE with STREAM to keep iterating, and the server responds to STREAM with a 2nd DONE message.
This is expected and should be handled correctly (ie: ignored) by the client.
| @@ -60,7 +60,6 @@ private boolean fetchAndCheck() { | |||
| case STREAM_RES_PART: | |||
| switch (resPart.getStreamResPart().getState()) { | |||
| case DONE: | |||
| stream.iteratorDone(requestID); | |||
There was a problem hiding this comment.
do we still need the bidirectional stream in this class at all?
flyingsilverfin
approved these changes
Jan 27, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What is the goal of this PR?
We no longer delete response collectors in a transaction after receiving a response to a "single" request, or receiving a "DONE" message in a stream. This fixes a possible error when loading 50+ answers in one query and then performing a second query.
What are the changes implemented in this PR?
See typedb/typedb-driver-python#250, which this PR is a copy of.