Adjust transaction lifetime tests for Python #598
Merged
Conversation
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
Currently, the tests assert that the driver throws the same error with which the other result failed. I personally don't like it because that error has nothing to do with the result stream being used (but only the other failed stream). So instead I decided to throw some generic `ResultFailedError` in Python with the other error as context information for easier debugging. However, all of this is fool proofing of the drivers. Users will never encounter this error unless they're abusing the driver or doing really funky stuff with it. So I don't have a strong opinion about what exact error we should throw. The main focus should be on preventing the driver from sending invalid messages to the server and ideally throwing any error that's not absolutely cryptic.
robsdedude
commented
Oct 5, 2023
| @@ -343,7 +341,7 @@ def test_should_prevent_run_after_tx_termination_on_run(self): | |||
| self._assert_is_client_exception(exc) | |||
|
|
|||
| with self.assertRaises(types.DriverError) as exc: | |||
| tx.run("invalid") | |||
| tx.run("RETURN 1 AS n") | |||
There was a problem hiding this comment.
Note this change.
I felt it was pointless to send a query here which we know will fail with exactly that Syntax error. The driver should throw even before attempting to send anything.
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.
Currently, the tests assert that the driver throws the same error with which the other result failed. I personally don't like it because that error has nothing to do with the result stream being used (but only the other failed stream). So instead I decided to throw some generic
ResultFailedErrorin Python with the other error as context information for easier debugging.However, all of this is fool proofing of the drivers. Users will never encounter this error unless they're abusing the driver or doing really funky stuff with it. So I don't have a strong opinion about what exact error we should throw. The main focus should be on preventing the driver from sending invalid messages to the server and ideally throwing any error that's not absolutely cryptic.