From de568ea50f0e15ffac37438b053a7f565a3bd8ca Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 15 Jun 2017 11:12:57 +0100 Subject: [PATCH 1/7] Deprecate "omego upgrade" and initdb upgradedb flags Replaced by "install --upgrade" and managedb flag --- omego/upgrade.py | 96 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 11 deletions(-) diff --git a/omego/upgrade.py b/omego/upgrade.py index 2df70a4..bfd0ad8 100644 --- a/omego/upgrade.py +++ b/omego/upgrade.py @@ -1,6 +1,7 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- +import argparse import copy import os import shutil @@ -21,22 +22,22 @@ class Install(object): def __init__(self, cmd, args): - - self.args = args + self.args, newinstall = self._handle_args(cmd, args) log.info("%s: %s", self.__class__.__name__, cmd) log.debug("Current directory: %s", os.getcwd()) self.symlink_check_and_set() - if cmd == 'upgrade': - newinstall = False + if newinstall is None: + # Automatically install or upgrade + newinstall = not os.path.exists(args.sym) + elif newinstall is False: if not os.path.exists(args.sym): raise Stop(30, 'Symlink is missing: %s' % args.sym) - elif cmd == 'install': - newinstall = True + elif newinstall is True: if os.path.exists(args.sym): raise Stop(30, 'Symlink already exists: %s' % args.sym) else: - raise Exception('Unexpected command: %s' % cmd) + assert False server_dir = self.get_server_dir() @@ -71,6 +72,64 @@ def __init__(self, cmd, args): self.external.save_env_vars(args.savevarsfile, args.savevars.split()) self.start() + def _handle_args(self, cmd, args): + """ + We need to support deprecated behaviour for now which makes this + quite complicated + + Current behaviour: + - install: Installs a new server, existing server causes an error + - install --upgrade: Installs or upgrades a server + - install --managedb: Automatically initialise or upgrade the db + + Deprecated: + - install --upgradedb --initdb: Replaced by install --managedb + - install --upgradedb: upgrade the db, must exist + - install --initdb: initialise the db + - upgrade: Upgrades a server, must already exist + - upgrade --upgradedb: Automatically upgrade the db + + returns: + - Modified args object, flag to indicate new/existing/auto install + """ + if cmd == 'install': + if args.managedb: + # Current behaviour + if args.initdb or args.upgradedb: + raise Stop(10, ( + 'Deprecated --initdb --upgradedb flags ' + 'are incompatible with --managedb')) + args.initdb = True + args.upgradedb = True + else: + # Deprecated behaviour + pass + + if args.upgrade: + # Current behaviour: install or upgrade + if args.initdb or args.upgradedb: + raise Stop(10, ( + 'Deprecated --initdb --upgradedb flags ' + 'are incompatible with --upgrade')) + newinstall = None + else: + # Current behaviour: Server must not exist + newinstall = True + + elif cmd == 'upgrade': + # Deprecated behaviour + log.warn( + '"omero upgrade" is deprecated, use "omego install --upgrade"') + cmd = 'install' + args.upgrade = True + # Deprecated behaviour: Server must exist + newinstall = False + + else: + raise Exception('Unexpected command: %s' % cmd) + + return args, newinstall + def get_server_dir(self): """ Either downloads and/or unzips the server if necessary @@ -209,6 +268,9 @@ def handle_database(self): status = db.check() log.debug('OMERO database upgrade status: %s', status) + # TODO: When initdb and upgradedb are dropped we can just test + # managedb, but for backwards compatibility we need to support + # initdb without upgradedb and vice-versa if status == DB_INIT_NEEDED: if self.args.initdb: log.debug('Initialising OMERO database') @@ -460,8 +522,10 @@ def __call__(self, args): class InstallCommand(InstallBaseCommand): """ - Setup a new OMERO installation. + Setup or upgrade an OMERO installation. """ + # TODO: When UpgradeCommand is removed InstallBaseCommand and + # InstallCommand can be combined NAME = "install" @@ -469,14 +533,24 @@ def __init__(self, sub_parsers): super(InstallCommand, self).__init__(sub_parsers) group = self.parser.add_argument_group('Database management') group.add_argument("--initdb", action="store_true", - help="Initialise the database if necessary") + help=argparse.SUPPRESS) group.add_argument("--upgradedb", action="store_true", - help="Upgrade the database if necessary") + help=argparse.SUPPRESS) + self.parser.add_argument( + "--upgrade", action="store_true", + help="Upgrade the server if already installed") + self.parser.add_argument( + "--managedb", action="store_true", + help="Initialise or upgrade the database if necessary") + self.parser.add_argument( + "--archivelogs", default=None, help=( + "If a logs directory exists archive to this zip file, " + "overwriting if it exists")) class UpgradeCommand(InstallBaseCommand): """ - Upgrade an existing OMERO installation. + DEPRECATED: Use `omego install --upgrade` instead """ NAME = "upgrade" From 48b1a398ad563426e444a13959562c2738944fb0 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 15 Jun 2017 11:27:15 +0100 Subject: [PATCH 2/7] Unit test for deprecated args --- test/unit/test_upgrade.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/unit/test_upgrade.py b/test/unit/test_upgrade.py index cb2f41d..348376d 100644 --- a/test/unit/test_upgrade.py +++ b/test/unit/test_upgrade.py @@ -66,6 +66,21 @@ def setup_method(self, method): def teardown_method(self, method): self.mox.UnsetStubs() + @pytest.mark.parametrize('cmd,expected', [ + ('install', True), + ('upgrade', False), + ]) + def test_handle_args_deprecated(self, cmd, expected): + args = self.Args({ + 'initdb': False, + 'upgradedb': False, + 'managedb': False, + 'upgrade': False, + }) + upgrade = self.PartialMockUnixInstall(args, None) + args, newinstall = upgrade._handle_args(cmd, args) + assert newinstall is expected + @pytest.mark.parametrize('server', [None, 'local', 'remote']) def test_get_server_dir(self, server): ext = self.mox.CreateMock(External) From 1696d9db6058819e6d38821aebe192ad8081d3ea Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 15 Jun 2017 12:35:35 +0100 Subject: [PATCH 3/7] Fix error message when db out of date --- omego/upgrade.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omego/upgrade.py b/omego/upgrade.py index bfd0ad8..645f7f4 100644 --- a/omego/upgrade.py +++ b/omego/upgrade.py @@ -288,7 +288,7 @@ def handle_database(self): else: raise Stop( DB_UPGRADE_NEEDED, - 'Pass --upgradedb or upgrade your OMERO database manually') + 'Pass --managedb or upgrade your OMERO database manually') else: assert status == DB_UPTODATE From 5f9a1274f5a884d001a5adaf0697a6c615d7e3fc Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 15 Jun 2017 12:35:20 +0100 Subject: [PATCH 4/7] update .travis to use install --upgrade. check db status --- travis-build | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/travis-build b/travis-build index 916ec97..eac154c 100755 --- a/travis-build +++ b/travis-build @@ -30,6 +30,9 @@ omego -h if [ $TEST = install ]; then omego install --initdb --dbhost localhost --dbname omero --prestartfile $HOME/config.omero -v --release 5.0.8 --ice 3.5 + # Should return 0 DB_UPTODATE + omego db upgrade -n --dbhost localhost --dbname omero --serverdir OMERO.server-5.0.8-ice35-b60 + # Check the expected server version was downloaded test $(readlink OMERO.server) = './OMERO.server-5.0.8-ice35-b60' @@ -38,13 +41,37 @@ if [ $TEST = install ]; then pg_restore -e travis-omero.pgdump | grep 'CREATE TABLE dbpatch' fi -#Test a multistage DB upgrade (4.4 -> 5.2) as part of the server upgrade +#Test a multistage DB upgrade (4.4 -> 5.3) as part of the server upgrade if [ $TEST = upgrade ]; then omego download --release 4 --ice 3.5 server ln -s OMERO.server-4.4.12-ice35-b116 OMERO.server; + + # Should return 3 DB_INIT_NEEDED + set +e + omego db upgrade -n --dbname omero --serverdir OMERO.server + RC=$? + set -e + test $RC -eq 3 + OMERO.server/bin/omero db script "" "" ome -f OMERO.sql; psql -q -h localhost -U omero omero < OMERO.sql; OMERO.server/bin/omero load $HOME/config.omero; OMERO.server/bin/omero admin start; - omego upgrade --release=5.2 --ice 3.5 --upgradedb --no-web -v; + + # Should return 0 DB_UPTODATE + omego db upgrade -n --serverdir OMERO.server + + omego download --release 5.3 --ice 3.5 server --sym download-server-50 + # Should return 2 DB_UPGRADE_NEEDED + set +e + omego db upgrade -n --dbname omero --serverdir download-server-50 + RC=$? + set -e + test $RC -eq 2 + + # Note this should use the already downloaded zip from the previous step + omego install --upgrade --managedb --release=5.3 --ice 3.5 --no-web; + + # Should return 0 DB_UPTODATE + omego db upgrade -n --serverdir OMERO.server fi From 46ce484bbd25e13e02901ea8de27f55f3a62e441 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 15 Jun 2017 12:49:56 +0100 Subject: [PATCH 5/7] Fix order of deprecated arg changes --- omego/upgrade.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/omego/upgrade.py b/omego/upgrade.py index 645f7f4..c02a8ec 100644 --- a/omego/upgrade.py +++ b/omego/upgrade.py @@ -93,6 +93,17 @@ def _handle_args(self, cmd, args): - Modified args object, flag to indicate new/existing/auto install """ if cmd == 'install': + if args.upgrade: + # Current behaviour: install or upgrade + if args.initdb or args.upgradedb: + raise Stop(10, ( + 'Deprecated --initdb --upgradedb flags ' + 'are incompatible with --upgrade')) + newinstall = None + else: + # Current behaviour: Server must not exist + newinstall = True + if args.managedb: # Current behaviour if args.initdb or args.upgradedb: @@ -105,17 +116,6 @@ def _handle_args(self, cmd, args): # Deprecated behaviour pass - if args.upgrade: - # Current behaviour: install or upgrade - if args.initdb or args.upgradedb: - raise Stop(10, ( - 'Deprecated --initdb --upgradedb flags ' - 'are incompatible with --upgrade')) - newinstall = None - else: - # Current behaviour: Server must not exist - newinstall = True - elif cmd == 'upgrade': # Deprecated behaviour log.warn( From 8feb348ac8975823729236a8dd8c7c1ef92edae8 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 15 Jun 2017 13:09:19 +0100 Subject: [PATCH 6/7] Avoid set +e/-e when testing omero db upgrade -n --- travis-build | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/travis-build b/travis-build index eac154c..f1572b1 100755 --- a/travis-build +++ b/travis-build @@ -47,10 +47,8 @@ if [ $TEST = upgrade ]; then ln -s OMERO.server-4.4.12-ice35-b116 OMERO.server; # Should return 3 DB_INIT_NEEDED - set +e - omego db upgrade -n --dbname omero --serverdir OMERO.server - RC=$? - set -e + RC=0; + omego db upgrade -n --dbname omero --serverdir OMERO.server || RC=$? test $RC -eq 3 OMERO.server/bin/omero db script "" "" ome -f OMERO.sql; @@ -63,10 +61,8 @@ if [ $TEST = upgrade ]; then omego download --release 5.3 --ice 3.5 server --sym download-server-50 # Should return 2 DB_UPGRADE_NEEDED - set +e - omego db upgrade -n --dbname omero --serverdir download-server-50 - RC=$? - set -e + RC=0; + omego db upgrade -n --dbname omero --serverdir download-server-50 || RC=$? test $RC -eq 2 # Note this should use the already downloaded zip from the previous step From 62acb73ba7a02bfaef9278c6e37102e89ea882a4 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 15 Jun 2017 13:48:46 +0100 Subject: [PATCH 7/7] Warn if --initdb or --upgradedb used with omego install --- omego/upgrade.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/omego/upgrade.py b/omego/upgrade.py index c02a8ec..c7f4449 100644 --- a/omego/upgrade.py +++ b/omego/upgrade.py @@ -113,8 +113,9 @@ def _handle_args(self, cmd, args): args.initdb = True args.upgradedb = True else: - # Deprecated behaviour - pass + if args.initdb or args.upgradedb: + log.warn('--initdb and --upgradedb are deprecated, ' + 'use --managedb') elif cmd == 'upgrade': # Deprecated behaviour