qizhiyi / sqlalchemy-migrate

Automatically exported from code.google.com/p/sqlalchemy-migrate
MIT License
0 stars 0 forks source link

Transaction per full upgrade #80

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago

I wrote a little patch that set transaction on full db upgrade (patch is
attached and for hq rev 185).

Now in SA-migrate engine is connectionless so on each sql command it's
creating a connection, doing sql req and then commiting and closing
connection. So there was an autocommit. I sixed id by adding
strategy=thredlocal to engine. In actual version it can be passed from
commandline: python migrate.py upgrade --repository=./repo --url=URL
--engine_arg_strategy=threadlocal.
No we can use transaction per one patch (upgrade). Like this:

def upgrade(engine):
    try:
        engine.begin()
        ...
        engine.commit()
    except:
        engine.rollback()

To set transaction on full upgrade (all patches) I had to change
migrate.versioning.api._migrate funciton.

With attached patch transaction is default on. You can pass
--no-transaction to commandline (upgrade or downgrade command) to use
version without transaction.

Original issue reported on code.google.com by lukasz.z...@gmail.com on 19 Feb 2010 at 3:50

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by jan.ditt...@gmail.com on 19 Feb 2010 at 3:55

GoogleCodeExporter commented 8 years ago
my sa-migrate clone and patch are here
https://code.google.com/r/lukaszzukowski-sqlalchemy-migrate/.

Original comment by lukasz.z...@gmail.com on 19 Feb 2010 at 5:27

GoogleCodeExporter commented 8 years ago
Hey Lukasz,

It's undocumented at the moment but a refactoring in 0.5.6 broke transactions 
usage 
in migrate.

Connection strategy needs more studying from my part and quick conversation 
with 
zzzek before I am willing to advanced with this issue.

Here is a quick rundown what needs to be done for this ticket to close:

1. write test coverage for transaction usage with schema
2. write test coverate for transaction usage with API upgrade/downgrade/test
3. figure out best connection/engine handling (relevant to 
http://groups.google.com/group/migrate-users/browse_thread/thread/42a98e8cff521d
9e)
4. provide default transaction wrapping of upgrade/downgrade/test functions
5. document backward incompatible changes and transaction feature

Your patch solves partly point 4, thanks for pushing this issue one step closer 
to 
better migrate:)

Original comment by iElect...@gmail.com on 20 Feb 2010 at 12:59

GoogleCodeExporter commented 8 years ago
AD 1. I think that sqlite wil be a problem. I see that on sqlite CREATE TABLE 
is out
of transactio and rollback don't revert this changes (only inserts, uptdate 
etc..).
AD 2. yes, I was going to write this tests today
AD 3. dosc about strategy:
:param strategy='plain': used to invoke alternate 
:class:`~sqlalchemy.engine.base.Engine
        implementations. Currently available is the ``threadlocal``
        strategy, which is described in :ref:`threadlocal_strategy`.
I don't see any other strategy then plain and threadlocal. In plain you don't 
have
transactions, so threadlocal is only choice (and only documented)
http://www.sqlalchemy.org/docs/dbengine.html#using-the-threadlocal-execution-str
ategy
AD 4. Ok, same changes in test command and it's done.
AD 5. This docs should be in Changelog or other location?

Original comment by lukasz.z...@gmail.com on 22 Feb 2010 at 10:08

GoogleCodeExporter commented 8 years ago
Any updates on this? Right now it's very easy to write a bad migration that 
leaves the db in a very broken state, and requires a lot of manual hacking in 
the db to fix. Even a workaround would save my day.

Original comment by treyst...@gmail.com on 10 Dec 2010 at 12:35

GoogleCodeExporter commented 8 years ago
Transactional DDL is not supported by all databases (I only know that 
PostgreSQL is capable). I can pull Lukasz's patch though and if our CI build 
works without regressions we will at least have transactional DDL support for 
PostgreSQL.

Original comment by jan.ditt...@gmail.com on 28 Oct 2011 at 9:05