Skip to content
This repository was archived by the owner on Oct 9, 2023. It is now read-only.

Conversation

flyingsilverfin
Copy link
Member

@flyingsilverfin flyingsilverfin commented Mar 22, 2022

What is the goal of this PR?

Revert previous changes from #247 and #248, which made query queues and iterators throw the same error idempotently. However, this goes counter to standard usage of iterators and queues, which are not meant to behave idempotently (each item is only returned once, and if they have an error they should no longer be used).

What are the changes implemented in this PR?

  • remove idempotent error state of collectors and queues, which back query iterators
    • note that we still store the error on the transaction bidirectional stream, in case the server throws an exception when there are no query iterators active

Note: mirrors change from typedb/typedb-driver#372

Comment on lines -136 to -137
value = self._stream.fetch(self._request_id)
return value
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inline var

Comment on lines +62 to +65
elif response.is_done() and response.error is None:
raise TypeDBClientException.of(TRANSACTION_CLOSED)
elif response.is_done() and response.error is not None:
raise TypeDBClientException.of_rpc(response.error)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go back to the old behaviour of retrieving the error from the Done message stored in the queue, which will be thrown exactly once. After that, this queue will always throw transaction closed.

raise TypeDBClientException.of(ILLEGAL_STATE)

def __next__(self) -> transaction_proto.Transaction.ResPart:
if self._bidirectional_stream.get_error() is not None:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the error will come from the backing Queue, rather than the bidirectional stream.

@flyingsilverfin flyingsilverfin self-assigned this Mar 22, 2022
@alexjpwalker
Copy link
Member

alexjpwalker commented Mar 22, 2022

Revert previous changes from #247 and #247

@flyingsilverfin lol

Copy link
Member

@alexjpwalker alexjpwalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems identical to the Java client PR 👍

@flyingsilverfin flyingsilverfin merged commit 6282d1e into typedb:master Mar 22, 2022
@flyingsilverfin flyingsilverfin deleted the only-store-tx-error-on-bidistream branch March 22, 2022 11:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants