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

added context manager / decorator for transactions [WIP] #1087

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

beniwohli
Copy link
Contributor

What does this pull request do?

Related issues

closes #ISSUE

@apmmachine
Copy link
Contributor

apmmachine commented Apr 7, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-10-13T16:27:28.052+0000

  • Duration: 32 min 55 sec

  • Commit: e8917d9

Test stats 🧪

Test Results
Failed 0
Passed 10031
Skipped 8977
Total 19008

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

Copy link
Contributor

@basepi basepi left a comment

Choose a reason for hiding this comment

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

This looks awesome so far. What's left?

def __call__(self, func):
self.name = self.name or get_name_from_func(func)

@functools.wraps(func)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use wrapt here?

Copy link
Contributor

@kalemas kalemas Nov 19, 2022

Choose a reason for hiding this comment

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

Btw does it makes sense to PR on this PR? As this implementation lacks some useful cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kalemas You're welcome to make a PR, though this PR is pretty out of date so it might make more sense to wait until @beniwohli gets back to it.

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.

5 participants