Uh oh!
There was an error while loading. Please reload this page.
fix: Fix for flaky test_read_timestamp_client_side_autocommit test - #1071
Conversation
| assert isinstance(read_timestamp_query_result_1[0][0], DatetimeWithNanoseconds) | ||
| time.sleep(0.25) | ||
| self._cursor.execute("SELECT * FROM contacts") |
There was a problem hiding this comment.
Do you feel we need to test this command twice? This way we can avoid sleep, just thoughts.
Java does it only once and validates the type of result. https://github.com/googleapis/java-spanner/blob/58f94b200276d879f83e4432716b49baf3206226/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java#L1003
There was a problem hiding this comment.
Not sure I understand your comment clearly. We are executing command twice because we want to confirm that the state is updated in the cursor class and that read timestamps are different.
Also removed the sleep command and inserting a row after discussion with Knut
There was a problem hiding this comment.
nit: it should not be necessary to set the connection in read-only mode for this as long as we are in auto-commit mode. (Having it in read-only mode also does not hurt, so feel free to leave as-is, as long as we are confident that it would work with self._conn.read_only=False` as well.)
harshachinta
left a comment
There was a problem hiding this comment.
LGTM, but wondering why do we need to execute the command twice and verify the read_timestamp of both.
| assert isinstance(read_timestamp_query_result_1[0][0], DatetimeWithNanoseconds) | ||
| time.sleep(0.25) | ||
| self._cursor.execute("SELECT * FROM contacts") |
There was a problem hiding this comment.
nit: it should not be necessary to set the connection in read-only mode for this as long as we are in auto-commit mode. (Having it in read-only mode also does not hurt, so feel free to leave as-is, as long as we are confident that it would work with self._conn.read_only=False` as well.)
No description provided.