Skip to content

expose method is_in_transaction #297

New issue

Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “No 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? No Sign in to your account

Merged
merged 5 commits into from
May 30, 2018
Merged

expose method is_in_transaction #297

merged 5 commits into from
May 30, 2018

Conversation

bcaudell95
Copy link
Contributor

Added method checking the state of _top_xact.

I'm not familiar with the library's version number usage, so I just bumped the lowest element of the version number. Correct me if this is wrong.

@elprans
Copy link
Member

elprans commented May 29, 2018

self._protocol.is_in_transaction() is a better check as it accounts for transactions started with con.execute("BEGIN") as well. Please also add some tests. Thanks!

@bcaudell95
Copy link
Contributor Author

bcaudell95 commented May 29, 2018

@elprans Does self._protocol.is_in_transaction() also return True for transactions started in an async with block? Or should the desired condition be the or of the two?

@1st1
Copy link
Member

1st1 commented May 29, 2018

@elprans Does self._protocol.is_in_transaction() also return True for transactions started in an async with block? Or should the desired condition be the or of the two?

"or". But write a test first and see what's actually going on there.

@elprans
Copy link
Member

elprans commented May 29, 2018

I think we should always rely only on the protocol state. _top_xact should always be None if there is no server transaction running. If it's not, it's a bug.

@bcaudell95
Copy link
Contributor Author

Fair enough; the protocol state gets set when a BEGIN query goes through either from the Transaction class or via a manual transaction, correct?

Can someone with more travis-ci experience than me explain why https://travis-ci.org/MagicStack/asyncpg/jobs/385397537 failed? Looks like curl failed to get a python3.5 install, but it's not clear to me why.

@elprans
Copy link
Member

elprans commented May 29, 2018

the protocol state gets set when a BEGIN query goes through either from the Transaction class or via a manual transaction, correct?

Yes

Can someone with more travis-ci experience than me explain why https://travis-ci.org/MagicStack/asyncpg/jobs/385397537 failed?

Looks like a Travis fluke. I restarted the job.

def is_in_transaction(self):
"""Return True if Connection is currently inside a transaction

:return bool: True if inside transaction, False otherwise
Copy link
Member

Choose a reason for hiding this comment

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

Please add :versionadded: 0.16.0 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

syntax you provided was slightly different than that I found elsewhere in the file, correct me if this is wrong

@@ -204,6 +204,14 @@ def transaction(self, *, isolation='read_committed', readonly=False,
self._check_open()
return transaction.Transaction(self, isolation, readonly, deferrable)

def is_in_transaction(self):
"""Return True if Connection is currently inside a transaction
Copy link
Member

Choose a reason for hiding this comment

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

Add a . at the end of all sentences please.

@1st1 1st1 merged commit cf523be into MagicStack:master May 30, 2018
@1st1
Copy link
Member

1st1 commented May 30, 2018

Thank you!

No Sign up for free to join this conversation on GitHub. Already have an account? No Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants