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

Context-manager for transactions with Skeletons with MySkel().transact(): ... #991

Closed
phorward opened this issue Dec 12, 2023 · 4 comments
Closed
Labels
feature New feature or request idea proposal

Comments

@phorward
Copy link
Member

It would be nice to do something like this:

with self.editSkel().transact(key) as skel:
    skel["counter"] += 1

Currently, it is impossible, as viur-datastore needs to be enhanced.

@phorward phorward added feature New feature or request proposal idea labels Dec 12, 2023
@phorward phorward added this to the ViUR-core v4.0 milestone Dec 12, 2023
@phorward
Copy link
Member Author

Relates to #853

@ArneGudermann
Copy link
Contributor

I did a bit of research. The problem with the context managers is that you can't execute the code to the context repeatedly. However, this may be necessary because the transaction fails.
An example:

def tnx(key):
    obj = db.Get(key)
    obj["count"]+=1
    db.Put(obj)

This function runs in a transaction and the transaction fails with a CollisionError, then the viur-datastore waits 2 seconds and then tries again and executes the function again so that we can count accurately.

But when we have code like:

db.Transaction():
    obj = db.Get(key)
    obj["count"]+=1
    db.Put(obj)

And here the the transaction fails we can not rerun the fuction.

If there is a solution to this problem, it would be nice, but I haven't found one.

@phorward
Copy link
Member Author

Hello @ArneGudermann, thanks for taking a closer look on this.

IMHO this could be achieved by modifying and/or modularization of code in viur-datastore, around here: https://github.com/viur-framework/viur-datastore/blob/master/src/viur/datastore/transport.pyx#L806
There is a beginTransaction and a commit, which must be turned into the ContextManager object.

Nevertheless, I don't want to heavily change viur-datastore for features in viur-core, as we still plan to integrate other database adapters in far future.

As an alternative for this issue, this is my most-current version of the set_status function I'm using throughout several projects:

"""
This module contains some project-specific helper functions.
"""

import logging
import time
from viur.core import db, errors


def set_status(key, values=None, check=None, func=None, skel=None, retry=3, update_relations=False):
    """
    Universal function to set a status of an db.Entity or Skeleton within a transaction.

    :param key: Entity/skeleton key to change
    :param values: A dict of key-values to update on the entry
    :param check: An optional dict of key-values to check on the entry before
    :param func: A function that is called inside the transaction
    :param skel: Use assigned skeleton instead of low-level DB-API
    :param retry: On BadRequestError, retry for this amount of times.
    :param update_relations: Instruct ViUR to update relations (normally not required)

    If the function does not raise an Exception, all went well.
    It returns either the assigned skel, or the db.Entity on success.
    """
    if callable(values):
        assert not func, "'values' is a callable, but func is also set. Either set values or func in this case."
        func = values
        values = None
    else:
        assert isinstance(values, dict) or values is None, "'values' has to be a dict when set"

    def transaction():
        if skel:
            if not skel.fromDB(key):
                raise errors.NotFound()

            obj = skel
        else:
            obj = db.Get(key)

        if check:
            if callable(check):
                assert check(obj)
            else:
                assert isinstance(check, dict), "'check' has to be a dict, you diggi!"

                for bone, value in check.items():
                    assert obj[bone] == value, "%r contains %r, expecting %r" % (bone, obj[bone], value)

        if values:
            for bone, value in values.items():
                if bone[0] == "+":
                    obj[bone[1:]] += value
                elif bone[0] == "-":
                    obj[bone[1:]] -= value
                else:
                    obj[bone] = value

        if func:
            assert callable(func)
            func(obj)

        if skel:
            assert skel.toDB(update_relations=update_relations)
        else:
            db.Put(obj)

        return obj

    if not retry:
        retry = 1

    for i in range(retry):
        try:
            return db.RunInTransaction(transaction)

        except db.Error as e:
            logging.exception(e)
            logging.debug(f"Retry {i}")
            time.sleep(i)

        except Exception:
            raise

    raise errors.InternalServerError()

Integrated and attached to Skeleton, this could also be executed like skel.transact({"+count", 1}) or skel.transact(callback_func), which is much nicer and shorter than using db.RunInTransaction.

phorward added a commit that referenced this issue Oct 15, 2024
Provides a `Skeleton.patch()` function based on `viur.toolkit.db.set_status()`.

This improves the Skeleton API to a better and more integrated CRUD-API
(Create-Read-Update-Delete). It also introduces a
ReadFromClientException that can be raised internally for client parsing
error reporting.

This pull request is part of #630 and a follow-up on #1264, which
~should be~ was merged first.

This is also an alternative replacement on #991.

---------

Co-authored-by: agudermann <[email protected]>
Co-authored-by: Sven Eberth <[email protected]>
@phorward
Copy link
Member Author

Fixed by #1267

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request idea proposal
Projects
None yet
Development

No branches or pull requests

2 participants