Skip to content

Conversation

@mf2199
Copy link
Collaborator

@mf2199 mf2199 commented Jul 24, 2019

Addresses googleapis#8695.

The observed error 400 Invalid CreateSession request is thrown when the SnapshotCheckout() line session = self._session = self._database._pool.get() invokes BurstyPool.get() that tries to create a session under an obviously non-existing instance. The proposed way of solving it puts a conditional "check-point" statement just before constructing the SnapshotCheckout() object.

Since the BatchCheckout() class does exactly the same, the above may apply to the Database.batch() factory too. If so, it makes sense to encapsulate the conditional statement in a private helper function, to be called within each of the two factories.

@mf2199 mf2199 self-assigned this Jul 24, 2019
@mf2199 mf2199 marked this pull request as ready for review July 24, 2019 18:32
@mf2199 mf2199 requested review from IlyaFaer, emar-kar and sumit-ql July 24, 2019 18:33
Copy link
Collaborator

@IlyaFaer IlyaFaer left a comment

Choose a reason for hiding this comment

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

I wonder, if googlers will request tests, which will be checking error's message. Still, code looks okey

@mf2199 mf2199 changed the title Adding validation checkpoints inside batch() and snapshot() methods. [GCP-8695] Adding validation checkpoints inside batch() and snapshot() methods. Jul 31, 2019
@sumit-ql
Copy link

sumit-ql commented Aug 2, 2019

I think we need to add some test coverage around _validate() method, otherwise nox session "cover" will fail with coverage less than 100%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants