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

Deprecate atomic keyword from Engine.save, Engine.delete #138

Closed
numberoverzero opened this issue Jun 6, 2019 · 0 comments
Closed

Deprecate atomic keyword from Engine.save, Engine.delete #138

numberoverzero opened this issue Jun 6, 2019 · 0 comments
Assignees
Milestone

Comments

@numberoverzero
Copy link
Owner

I'd like to remove this from the api and migrate it to a pattern. This would be a moderate to large performance improvement for the majority of users (likely >95%) without adding too much complexity for those currently using atomic=True when calling Engine.delete or Engine.save. An audit of public repositories suggest almost no usage, and a pattern can be created to replicate existing behavior.

Usage Audit

From an audit of github dependents there are two repos using this keyword (sort of).

1. todo-sam-python

todo-sam-python depends on an unversioned bloop which performs an atomic delete after querying an item. To see which attributes are included in the atomic condition I checked the model and given that completed is the only attribute checked, the following change would be enough:

# original lines
todo_item = db.query(TodoItem, key=TodoItem.uuid == UUID(todo_id)).one()
db.delete(todo_item, atomic=True)

# modified
todo_item = db.query(TodoItem, key=TodoItem.uuid == UUID(todo_id)).one()
db.delete(todo_item, condition=TodoItem.completed==todo_item.completed)

2. dvox

dvox depends on an unversioned bloop that from the engines.py file suggests a pre 0.9 bloop with other keywords that don't exist today.

This use can be safely ignored.

Migrating to a Pattern

The pattern would be mostly equivalent to what bloop's doing behind the scenes today:

# add this function to the Patterns section of the docs
def atomic(obj):
    c = Condition()
    for column in obj.Meta.columns:
        value = getattr(obj, column.name, None)
        c &= (column == value)
    return c


# usual loading code
user = User(...)
engine.load(user)

# snapshot the user state immediately
condition = atomic(user)

# some modifications
user.verified = True
user.email = "new@email.com"

# use the atomic condition created above
engine.save(user, condition=condition)

Deprecation Timeline

In 2.4, Engine.save and Engine.delete would change the default value of the atomic=keyword to bloop.Sentinel("deprecated-false") which would be treated as the current default behavior of False. Any user-provided value that isn't this sentinel, including False, will trigger a warning that the keyword is going away in 3.0.

In 3.0 the atomic= keyword will be removed along with the corresponding internal classes and arguments.

Since there are breaking changes for #136 as well, I expect 3.0 to release approximately 3 months after 2.4.

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

No branches or pull requests

1 participant