Skip to content
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

Prevent nested transactions. #3789

Merged
merged 3 commits into from
Aug 10, 2017

Conversation

lukesneeringer
Copy link
Contributor

Fixes #3476.

/cc @vkedia

Note that there is still some limitation on how much we can stop people from firing a blowgun into their foot. Someone could conceivably have two database objects, for example. But this is well within the limits of what we need to check for, in my opinion, especially given that you have to do pretty unusual stuff to even get this far.

@lukesneeringer lukesneeringer added the api: spanner Issues related to the Spanner API. label Aug 10, 2017
@lukesneeringer lukesneeringer self-assigned this Aug 10, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 10, 2017
@@ -79,6 +79,7 @@ def __init__(self, database_id, instance, ddl_statements=(), pool=None):
self.database_id = database_id
self._instance = instance
self._ddl_statements = _check_ddl_statements(ddl_statements)
self._transaction_running = False

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@lukesneeringer
Copy link
Contributor Author

@vkedia If this works for you, I will merge it.

@lukesneeringer lukesneeringer merged commit edc7c27 into googleapis:master Aug 10, 2017
@lukesneeringer lukesneeringer deleted the issue-3476 branch August 10, 2017 22:09
@tseaver
Copy link
Contributor

tseaver commented Aug 10, 2017

FWIW, this seems like a bunch of fragile mechanism to prevent (incompletely) a very rare kind of user error.

@lukesneeringer
Copy link
Contributor Author

Yeah, I have a hard time seeing a situation where a user does this by accident, even before this check was in place.

@vkedia
Copy link

vkedia commented Aug 11, 2017

There are databases that support nested transactions so it is not all the inconcievable that some user might expect cloud spanner to behave in the same way. Before this check, they would start using nested transactions and nothing would fail since as far as spanner is concerned these are independent transactions. They might incorrectly assume that this is actually working as a nested transaction. That is why it is worthwhile to have this check even if it does not work in all cases.

landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 21, 2017
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
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