Merge lp:~therp-nl/openupgrade-server/7.0_migrate_script into lp:openupgrade-server

[Holger Brunn (Therp)]

[ADD] migration call for 7.0
[IMP] use lightweight checkouts instead of branches, that saves a lot of
time and space
[IMP] always use versioned lp urls
[IMP] make migrate script executable

updated migrate.py

[Stefan Rijnhart (Opener) (stefan-opener)]

Good! Could you please take out the reference to banking-addons, which I believe should be part of your local configuration.

review: Needs Fixing

[lp:~therp-nl/openupgrade-server/7.0_migrate_script]

4613. By Holger Brunn (Therp) on 2013-05-07

[IMP] remove banking addons from default update code
[IMP] give banking addons as an example on how to add update code
[FIX] typos in help texts

[Holger Brunn (Therp) (hbrunn)]

Thanks, I took this as an opportunity to elaborate on the help text for -A

[Stefan Rijnhart (Opener) (stefan-opener)]

Testing your script as I need to upgrade a database. I downloaded the script separately as follows

bzr cat -d lp:~therp-nl/openupgrade-server/7.0_migrate_script scripts/migrate.py > migrate.py
./migrate.py -D diflbv -C ../../diflbv.cfg -R 6.1 -A add.py

Contents of add.py:

{'6.1': {'addons': {'banking': 'lp:banking-addons/6.1'}}}

This gives the following exception:

  File "./migrate.py", line 115, in <module>
    merge_migrations=merge_migrations_mod.migrations
AttributeError: 'module' object has no attribute 'migrations'

Line 113 attempts to load a python module called 'merge_migrations_mod' which I do not have because I downloaded the script as a standalone file but which is not present in the bzr branch either. Did you forget to push a module at some point?

review: Needs Information

[Holger Brunn (Therp) (hbrunn)]

your add.py needs to declare a global variable named 'migrations'.

So make a
migrations = {'6.1': {'addons': {'banking': 'lp:banking-addons/6.1'}}}
of it and it should work fine.

Or pass
-A "{'6.1': {'addons': {'banking': 'lp:banking-addons/6.1'}}}"

[Holger Brunn (Therp) (hbrunn)]

btw: the first parameter of imp.load_source is an arbitrary name how you want to call the module you import

[Stefan Rijnhart (Opener) (stefan-opener)]

Thanks for the info! It works when I change the contents of add.py accordingly.

Small glitch that needs to be repaired in the 6.1 definitions:

--- scripts/migrate.py 2013-05-07 06:46:24 +0000
+++ scripts/migrate.py 2013-05-09 08:32:59 +0000
@@ -34,7 +34,7 @@
           'web': {'url': 'lp:openerp-web/6.1', 'addons_dir': 'addons'},
         },
       'server': {
- 'url': 'lp:openupgrade-server',
+ 'url': 'lp:openupgrade-server/6.1',
           'addons_dir': os.path.join('openerp','addons'),
           'root_dir': os.path.join(''),
           'cmd': 'openerp-server --update=all --database=%(db)s '+

Using lightweight checkouts makes the script much faster! That is great for most uses. However, I cannot merge in any pending improvements anymore as a user without write access to the Launchpad series branch:

bzr merge lp:~therp-nl/openupgrade-addons/6.1-lp1168936-tax_code_sequence
bzr: ERROR: Cannot lock LockDir(http://bazaar.launchpad.net/~openupgrade-committers/openupgrade-addons/6.1/.bzr/branch/lock): Transport operation not possible: http does not support mkdir()

I can then replace the lightweight checkout by a branch and proceed. The migrate script will run, but if I reuse the migration directory later on, the branch will not get updated with the latest upstream revisions because the script performs an 'update', not a 'pull'.

My suggestion would be as follows:
- Optionally allow to get the code using a branch instead of a lightweight checkout
- When updating the code, use either pull or update depending on whether you are dealing with a branch or a checkout

What do you think?

review: Needs Information

[lp:~therp-nl/openupgrade-server/7.0_migrate_script]

4614. By Holger Brunn (Therp) on 2013-05-09

[FIX] also use correct version for openupgrade-server 6.1

[Holger Brunn (Therp) (hbrunn)]

Thanks for the pointer on the server url!

Concerning the pull/update issue: I think adding another commandline switch serves mostly to confuse a user. If you want to do development in this directories, you can say
bzr reconfigure --branch #yes, that's necessary because making it a tree directly crashes
bzr reconfigure --tree
[hackhackhack]
bzr commit
bzr push [your location]
bzr bind [original location]
So I'd rather leave it to the developer to do that.

[Stefan Rijnhart (Opener) (stefan-opener)]

OK, we will leave it at that then. I can pull upstream revisions manually if I reuse an existing branch.

review: Approve

=== modified file 'scripts/migrate.py' (properties changed: -x to +x)
--- scripts/migrate.py	2012-11-24 22:22:00 +0000
+++ scripts/migrate.py	2013-05-09 13:52:27 +0000
@@ -15,14 +15,26 @@
 import bzrlib.info
 
 migrations={
+  '7.0': {
+      'addons': {
+          'addons': 'lp:openupgrade-addons/7.0',
+          'web': {'url': 'lp:openerp-web/7.0', 'addons_dir': 'addons'},
+        },
+      'server': {
+          'url': 'lp:openupgrade-server/7.0', 
+          'addons_dir': os.path.join('openerp','addons'),
+          'root_dir': os.path.join(''),
+          'cmd': 'openerp-server --update=all --database=%(db)s '+
+            '--config=%(config)s --stop-after-init --no-xmlrpc --no-netrpc',
+        },
+    },
   '6.1': {
       'addons': {
-          'addons': 'lp:openupgrade-addons',
-          'banking': 'lp:banking-addons',
+          'addons': 'lp:openupgrade-addons/6.1',
           'web': {'url': 'lp:openerp-web/6.1', 'addons_dir': 'addons'},
         },
       'server': {
-          'url': 'lp:openupgrade-server', 
+          'url': 'lp:openupgrade-server/6.1', 
           'addons_dir': os.path.join('openerp','addons'),
           'root_dir': os.path.join(''),
           'cmd': 'openerp-server --update=all --database=%(db)s '+
@@ -32,7 +44,6 @@
   '6.0': {
       'addons': {
           'addons': 'lp:openupgrade-addons/6.0',
-          'banking': 'lp:banking-addons/6.0',
         },
       'server': {
           'url': 'lp:openupgrade-server/6.0',
@@ -62,12 +73,15 @@
         default='/var/tmp/openupgrade')
 parser.add_option("-R", "--run-migrations", action="store", type="string", 
         dest="migrations", 
-        help="comma separated list of migrations to run\n\n"+
+        help="comma separated list of migrations to run, ie. \""+
                 ','.join(sorted([a for a in migrations]))+
-                "\n(required)")
+                "\" (required)")
 parser.add_option("-A", "--add", action="store", type="string", dest="add",
-        help="load a python module that declares on dict 'migrations' which is"+
-        " merged with the one of this script (see the source for details)")
+        help="load a python module that declares a dict 'migrations' which is "+
+        "merged with the one of this script (see the source for details). "
+        "You also can pass a string that evaluates to a dict. For the banking "
+        "addons, pass "
+        "\"{'6.1': {'addons': {'banking': 'lp:banking-addons/6.1'}}}\"")
 parser.add_option("-I", "--inplace", action="store_true", dest="inplace",
         help="don't copy database before attempting upgrade (dangerous)")
 (options, args) = parser.parse_args()
@@ -148,13 +162,13 @@
                 name))
             print 'updating %s rev%s' %(os.path.join(version,name),
                     cmd_revno.outf.getvalue().strip())
-            cmd_pull=bzrlib.builtins.cmd_pull()
-            cmd_pull.outf=StringIO.StringIO()
-            cmd_pull.outf.encoding='utf8'
-            cmd_pull.run(directory=os.path.join(options.branch_dir,version,
-                name), overwrite=True)
-            if hasattr(cmd_pull, '_operation'):
-                cmd_pull.cleanup_now()
+            cmd_update=bzrlib.builtins.cmd_update()
+            cmd_update.outf=StringIO.StringIO()
+            cmd_update.outf.encoding='utf8'
+            cmd_update.run(dir=os.path.join(options.branch_dir,version,
+                name))
+            if hasattr(cmd_update, '_operation'):
+                cmd_update.cleanup_now()
             print 'now at rev'+cmd_revno.outf.getvalue().strip()
         else:
             if link:
@@ -163,10 +177,10 @@
                 os.symlink(url, os.path.join(options.branch_dir,version,name))
             else:
                 print 'getting '+url
-                cmd_branch=bzrlib.builtins.cmd_branch()
-                cmd_branch.outf=StringIO.StringIO()
-                cmd_branch.run(url, os.path.join(options.branch_dir,version,
-                    name))
+                cmd_checkout=bzrlib.builtins.cmd_checkout()
+                cmd_checkout.outf=StringIO.StringIO()
+                cmd_checkout.run(url, os.path.join(options.branch_dir,version,
+                    name), lightweight=True)
 
 if not options.inplace:
     print('copying database %(db_name)s to %(db)s...' % {'db_name': db_name,