Skip to content

Conversation

larkee
Copy link
Contributor

@larkee larkee commented Nov 6, 2019

TransactionPingingPool is throwing error ''NoneType' object is not callable' when trying to batch insert. This is the fix.

@larkee larkee added the api: spanner Issues related to the Spanner API. label Nov 6, 2019
@larkee larkee requested a review from crwilcox November 6, 2019 09:21
@larkee larkee self-assigned this Nov 6, 2019
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 6, 2019
Copy link
Contributor

@tseaver tseaver left a comment

Choose a reason for hiding this comment

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

Rather than tweak the hand-rolled _Transaction mock, let's take this as an opportunity to replace it with an "mock autospec" version, which will auttomatically keep the signatures correct.

@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 6, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 6, 2019
@larkee
Copy link
Contributor Author

larkee commented Nov 7, 2019

The mock autospec does not support static attributes which causes test_put_non_full_w_active_txn to fail because txn.committed is a NonCallableMagicMock, not None. I don't see an easy fix for that.

Perhaps, it would be easier to go with the original change and have this refactor in a subsequent PR?

@tseaver
Copy link
Contributor

tseaver commented Nov 7, 2019

@larkee Hmm, odd: I pasted in that change after making it locally, and got all the tests to pass. Nevertheless, we can update the _make_transaction() factory to assign txn.committed = None (which is an appropriate initial condition).

@larkee larkee requested a review from tseaver November 8, 2019 03:45
@larkee
Copy link
Contributor Author

larkee commented Nov 11, 2019

@tseaver PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants