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
Atomic transaction? #868
Comments
In that example, none of the queries should get committed. Is this Postgres or SQLite? If you give this a go in the playground ( async with Band._meta.db.transaction():
await Band.insert(Band(name='test'))
raise Exception() It doesn't get committed - check using this: await Band.select() Any exception raised within the async context manager should stop the transaction. Things to note though: async with Band._meta.db.transaction():
await Band.insert(Band(name='test'))
# This query sees the new Band, even though it's not committed:
print(await Band.select())
# This won't see the new Band, as the transaction has been rolled back.
print(await Band.select()) Something else to bear in mind is maybe there are nested async context managers, which is causing weirdness. async with DB.transaction():
async with DB.transaction():
... Other than that I'm not sure - maybe a strange bug somehow. |
Hm, when I use
I thought |
Yes, >>> from piccolo_conf import DB
>>> MyTable._meta.db is DB
True If they're not, then that explains why the transactions aren't getting rolled back. You can check the config for each one to see what they're pointing to: >>> from piccolo_conf import DB
>>> DB.config
{'database': 'my_app', ...}
>>> MyTable._meta.db.config
{'database': 'my_app', ...} |
They both point to the same config, but they are somehow still not the same object. I can reproduce with code like the following: from piccolo.columns.column_types import Varchar
from piccolo.engine.postgres import PostgresEngine
from piccolo.table import Table
from starlette.applications import Starlette
import uvicorn
import os
os.environ['PICCOLO_CONF'] = 'app'
DB = PostgresEngine(config={
# ..
})
class Band(Table):
name = Varchar(unique=True)
async def do_db_stuff():
print(Band._meta._db.config)
print(DB.config)
assert DB is Band._meta._db # throws
async with DB.transaction():
await Band.insert(Band(name="test1"))
assert False
await Band.insert(Band(name="test2"))
async def on_startup():
await do_db_stuff()
os._exit(0)
app = Starlette(debug=True, on_startup=[on_startup])
if __name__ == '__main__':
Band.alter().drop_table(if_exists=True).run_sync()
Band.create_table(if_not_exists=True).run_sync()
uvicorn.run(app, host='127.0.0.1', port=64216) |
I gave this a go, and you're right - in that situation I think the problem is that So it's some weirdness with Python's import mechanics when using WorkaroundMove DB into
|
module = t.cast(PiccoloConfModule, import_module(module_name)) |
What do you think?
If there is no ambiguity, I'd prefer swapping it instead of raising an error/warning. Maybe it would be possible to assert that only one DB Instance (with a particular config) got initiated? That way we could catch such "Module got instantiated multiple times" Bugs. Perhaps by hashing the config, and storing it in a piccolo internal set? Hm, maybe raising an warning when |
With code like this:
Band "test1" is still inserted. Is there a way to make the transaction "really atomic"? And what does the current transaction mechanism do, e.g. what is the difference to not using a transaction (in piccolo)? Thank you!
The text was updated successfully, but these errors were encountered: