-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(datastore): adds snapshot reads #6755
Conversation
datastore/datastore_test.go
Outdated
} | ||
|
||
ctx := context.Background() | ||
client.WithReadOptions(ReadTime(time.Now())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are we checking that the read time is actually sent to the backend? Maybe I'm missing something, but I don't see how this test exercises that logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a unit test. Any check where we read the response's read time would be merely a result of how we set up the mock (and hence not useful as a test case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is really just to ensure that using Client.WithReadOptions()
doesn't result in an error.
This PR adds snapshot reads to the client and transactions. Snapshot reads allow for a time-specific read-only version of the database to be accessed.
Note: This PR does not include integration tests, as read results do not indicate a read time.