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

Add support for transactions #53

Closed
1 of 2 tasks
Tracked by #116
RobertCraigie opened this issue Aug 26, 2021 · 28 comments · Fixed by #613
Closed
1 of 2 tasks
Tracked by #116

Add support for transactions #53

RobertCraigie opened this issue Aug 26, 2021 · 28 comments · Fixed by #613
Labels
kind/feature A request for a new feature. level/advanced priority/high process/candidate Candidate for the next release
Projects

Comments

@RobertCraigie
Copy link
Owner

RobertCraigie commented Aug 26, 2021

Prisma has preview support for interactive transactions.

The base work for this has already been done in the wip/transactions branch, some kinks need to be ironed out.

Status

  • Model-based access support
  • Protect against namespace collision

Model-based access

We could refactor the internal strategy to register and retrieve clients to use contextvars. However we should only use contextvars for transactions as otherwise it would break certain async use cases like the python -m asyncio REPL.

@RobertCraigie RobertCraigie added the kind/feature A request for a new feature. label Aug 26, 2021
@RobertCraigie RobertCraigie added this to To do in v1.0.0 via automation Aug 26, 2021
@RobertCraigie RobertCraigie added this to the v0.3.2 milestone Nov 19, 2021
@RobertCraigie RobertCraigie added process/candidate Candidate for the next release and removed process/candidate Candidate for the next release labels Nov 28, 2021
@RobertCraigie RobertCraigie removed this from the v0.4.1 milestone Nov 30, 2021
@danielweil
Copy link

danielweil commented Dec 20, 2021

Thanks for the great work. Is this still a work on progress? I would love to use this in my project

@RobertCraigie
Copy link
Owner Author

Yes this is still a work in progress, I'll update the WIP to the latest version and then you can try it out!

@danielweil
Copy link

Thanks!

@RobertCraigie
Copy link
Owner Author

@danielweil Just updated my WIP, you can try it out in your project by installing from the branch like so:

pip install -U git+https://github.com/RobertCraigie/prisma-client-py@wip/transactions

However, there are absolutely no guarantees this will actually work.

@danielweil
Copy link

Thanks, will try it out.

@RobertCraigie
Copy link
Owner Author

Let me know if you encounter any issues!

@danielweil
Copy link

I tried installing it with the command, without uninstalling my latest prisma version. It resulted in the error below:

image

@RobertCraigie
Copy link
Owner Author

I've never encountered that error before but it looks like it has something to do with setuptools.

Try running:

pip install -U pip
pip install -U setuptools

And then try again

@danielweil
Copy link

Worked by adding --no-use-pep517 to the pip command.

@danielweil
Copy link

Is there any docs commenting on how to use transactions? Or it is the same syntax as TS?

@RobertCraigie
Copy link
Owner Author

No docs yet, basically the same syntax as batching, here's an example:

async with client.tx() as transaction:
    user = await transaction.user.create({'name': 'Robert'})

@RobertCraigie
Copy link
Owner Author

You can have a look through the tests for more examples:

https://github.com/RobertCraigie/prisma-client-py/blob/wip/transactions/tests/test_transactions.py

@danielweil
Copy link

danielweil commented Dec 20, 2021

Thanks! I am facing an issue here, can you thing about it?

I use output in the schema file and outputs to './client'. WIth version 0.2.2, my module is being added correctly and it works importing it like this:

"from prisma.client import Client"

When I use the transaction version, after generating the client, it gives me the error:

Exception: ModuleNotFoundError: No module named 'prisma.client'. Troubleshooting Guide: https://aka.ms/functions-modulenotfound

Do you think there is something about it?

@RobertCraigie
Copy link
Owner Author

Are you sure you need to use a custom output in the first place? By default Prisma Client Python will generate itself to the same place it was installed to.

If you're importing from a custom output of './client' then you're going to need to import it like this:

from client import Client

@danielweil
Copy link

It outputs to:

Generated Prisma Client Python to ./prisma/client in 961ms

Sorry, trying to figure out here how to do it

@RobertCraigie
Copy link
Owner Author

Why are you using custom output?

You probably want to either remove custom output or change it to ../client.

@danielweil
Copy link

Yep, worked out this way! Thanks

Tried using tx and it gave me the following error:

'Client' object has no attribute 'tx'

I am using it this way:

image

@RobertCraigie
Copy link
Owner Author

@danielweil Have you added the preview feature flag to your schema? e.g.

generator client {
  provider               = "prisma-client-py"
  recursive_type_depth   = -1
  previewFeatures        = ["interactiveTransactions"]
}

@danielweil
Copy link

Worked Great!

@danielweil
Copy link

Robert, would you mind making a new branch where this work in progress in merged with the newest release?

@RobertCraigie
Copy link
Owner Author

@danielweil I've updated the wip/transactions branch to include the changes from the latest release.

@danigosa
Copy link

@danielweil I've updated the wip/transactions branch to include the changes from the latest release.

Hi, we are using internally this branch (not in production) for our platform to test out transactions, could you integrate the commits in master like this one #402 to forward the code to current?

@RobertCraigie
Copy link
Owner Author

@danigosa Done 👍

@danigosa
Copy link

For now we are using the interface TransactionManager without the context manager, we've seen so far that it works just fine, although we do not understand why the context manager version is not calling rollback on error.

This is how in our dev/staging environments is working with fastAPI with the standard DB middleware (open session/transaction on request, commit or rollback in response), calling explicitly tx_manager.rollback() to Prisma:

def get_db(request: Request) -> Prisma:
    """Global db fastApi dependency"""
    return request.state.db


async def db_tx_middleware(request: Request, call_next: Any) -> Response:
    """
    Middleware that creates a transaction per request/response cycle
    Every transaction is either commited or rollback on success/exception
    It stores state on `request.state` context store
    WIP: Preview TransactionManager feature in prisma-client-py@wip/transactions
    """
    response = Response("Internal server error", status_code=500)
    tx = prisma.tx()  # TransactionManager

    try:
        request.state.db = await tx.start()  # Client within the tx for the request
        response = await call_next(request)
    except Exception as e:
        log.exception(f"Exception while transaction, ROLLBACK: {e}")
        await tx.rollback()
    else:
        await tx.commit()

    return response
@router.get(
    "/organizations",
    response_model=List[OrganizationWithoutRelations],
)
async def list_organizations(
    take: int = 10, db: Prisma = Depends(get_db)
) -> List[Organization]:
    return await db.organization.find_many(take=take)

Background: we have a pgbouncer as data source for prisma in transaction mode, although we could live without transactions inferface batching writes, it made it hard to understand db operations like old fashion ORM, so this new interface allows us to operate as the good old request/response pattern typical in most ORM.

Some thoughts/questions:

  • Do you expect or consider to use TransactionManager out of a purely context manager?
  • Why the TransactionManager.__aexit__() do not call rollback when error?
  • Do you see any problem on working around the context manager interface this way?

@RobertCraigie
Copy link
Owner Author

@danigosa Thanks for the detailed comment!

Do you expect or consider to use TransactionManager out of a purely context manager?

Yes, I imagine that there may be cases where using a context manager is awkward but I can't think of any specific examples off the top of my head.

Why the TransactionManager.aexit() do not call rollback when error?

That appears to be a complete oversight on my part, thanks for pointing that out! While implementing this feature I was focused on getting the interactions with the engine to work instead of the actual feature behaviour. I'm grateful this was still a WIP as that is a pretty big oversight...

Do you see any problem on working around the context manager interface this way?

I can't see any problems occurring with usage outside of a context manager. The context manager simply provides a more convenient interface for handling exit / error state. What you have there is essentially the same as what the context manager will be doing (minus the missing rollback bug) and I can't foresee the actual interface, tx.start(), tx.commit() and tx.rollback() changing anytime soon.

@danigosa
Copy link

danigosa commented Jun 28, 2022

Thanks for the feedback!

The main reason we work around the context manager is because it wasn't calling rollback, I think that if this issue is addressed it can be re-written as simply:

async def db_tx_middleware(request: Request, call_next: Any) -> Response:
    """
    UNTESTED
    Middleware that creates a transaction per request/response cycle
    Every transaction is either commited or rollback on success/exception
    It stores state on `request.state` context store
    WIP: Preview TransactionManager feature in prisma-client-py@wip/transactions
    """
    async with prisma.tx() as tx:
           request.state.db = tx
           return await call_next(request)

And delegate error handling upstream in the framework chain.

For now the version using raw TransactionManager works like a charm, no issues at all opening and commiting/rolling back transactions

@caelx
Copy link

caelx commented Dec 12, 2022

I'm also very interested in using this feature has the rollback bug been fixed on the branch?

@RobertCraigie
Copy link
Owner Author

@caelx Unfortunately not. I will be working on this soon though as it is by far the most requested feature so far.

RobertCraigie added a commit that referenced this issue Jun 12, 2023
## Change Summary

closes #53

This PR also bumps the `typing-extensions` version for `NewType`
support.

TODO:
- [x] tests for behaviour wrt rollback
- [x] Check what `max_wait` does and document both options
- [x] Check if we need to bump the `typing-extensions` dep
- [x] Support for model based access
- [x] Fix synchronous tests
- [x] Docs (include timeout information) 

## Checklist

- [ ] Unit tests for the changes exist
- [ ] Tests pass without significant drop in coverage
- [ ] Documentation reflects changes where applicable
- [ ] Test snapshots have been
[updated](https://prisma-client-py.readthedocs.io/en/latest/contributing/contributing/#snapshot-tests)
if applicable

## Agreement

By submitting this pull request, I confirm that you can use, modify,
copy and redistribute this contribution, under the terms of your choice.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
v1.0.0 automation moved this from To do to Done Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A request for a new feature. level/advanced priority/high process/candidate Candidate for the next release
Projects
4 participants