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

omego migration #10

Merged
merged 34 commits into from
Aug 16, 2013
Merged

omego migration #10

merged 34 commits into from
Aug 16, 2013

Conversation

joshmoore
Copy link
Member

This rather large PR refactors the OMERO-qa-upgrade.py command in order to allow a wider variety of uses across the team. This first required extracting and slightly modifying the http://github.com/openmicroscopy/snoopycrimecop plugin infrastructure, which could later be further abstracted to its own project. CLI and ENV processing was all converted to use argparse, and packaging was added to support pypi/pip installation.

To test create a fresh virtualenv (adding argparse if Python 2.6 or lower) and run:

pip install git+ssh://git@github.com/joshmoore/omero-setup.git@omego-migration

The only available command at the moment is upgrade:

  • omego upgrade -h will list options
  • omego upgrade -v -n will list options with values
  • omego upgrade will perform a default installation of OMERO.

One of the first tasks will be to separate out init logic from upgrade so that it's clear whether the user was intending to upgrade an existing server or create a new location. Various other tasks are then planned: recording all installed servers (e.g. under ~/.omego), allowing data bootstrapping, etc.

/cc @ximenesuk @sbesson @manics @jburel @criswell @knabar @stick

See: https://trac.openmicroscopy.org.uk/ome/ticket/11339#comment:4

Basic setup.py and similar copied from the
http://github.com/openmicroscopy/snoopycrimecop
repository.
Requires the re-addition of argparse which was
previously removed from the files copied over.

From scc commit 67cd4b0ddff7f8ba74d6a16acfc1420675d61853
for diff'ing between scc.py and framework.py.
In order to combin @manics recent work with mine,
all environment and keyword variable parsing was
refactored to use argparse. Now, `-h` shows all
available settings and all are also settable via
the environment as before. Use `-v` to have values
printed to DEBUG.

Conflicts:
	omego/upgrade.py
argparse Action which can be used to also read values
from the current environment. Additionally, it will
replace any values in string replacement syntax that
have already be set in the environment (e.g. %(prefix)4064
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"been"

On a first installation, mklink is now called to point
OMERO-CURRENT (i.e. self.sym) at the self.dir location.
This satisfies all the requirements for lib/python etc
even though it is a bit counterintuitive.
Though it was an interesting way to try to reduce
character counts in the original OMERO-qa-upgrade.py
the passing of `_` as a variable is confusing to
say the least. Now using a simple `def run` method.
Remove unused imports and fix all error messages
from PyLint/PyFlakes. Rather than passing around
the individual arguments, now passing the single
`args` namespace.
@joshmoore
Copy link
Member Author

Note: travis is currently failing as I teach .travis.yml to properly setup for OMERO (PG, Ice, etc.)

@manics
Copy link
Member

manics commented Aug 15, 2013

Once joshmoore#1 is in and the README is fixed omego upgrade ... should be equivalent to OMERO-qa-upgrade.py ... and good to merge.

@joshmoore
Copy link
Member Author

Merged. @manics: worth waiting on the integration tests? And if so, can you jot down here the scenarios you're trying? It would be good to codify those under tests/.

@manics
Copy link
Member

manics commented Aug 15, 2013

Can do... though without tests we're no worse off than we were before.

Test scenario:

  1. Stop and delete existing servers.
  2. Create $HOME/config.xml (maybe the current directory would be a better location for this since we're autodetecting it- someone might inadvertently have one in their home dir)
  3. Set the initial ice33 environment:
    source env.sh ice33
  4. Install the initial server, check its running:
    omego upgrade --skipdelete false --branch OMERO-merge-develop
  5. Set the ice35 environment:
    source env.sh ice35
  6. Upgrade to a ice35 server, check its running:
    omego upgrade --skipdelete false --branch OMERO-merge-develop-ice35
  7. Upgrade back to the ice33 server:
    source env.sh ice33
    omego upgrade --skipdelete false --branch OMERO-merge-develop

env.sh (I was trying to make this a file which could be sourced with options to set the environment, but I couldn't find a way to make it return a non-zero code on error):

_omero_env() {
    case "$1" in
        ice33 )
        _ICE_HOME=/usr/local/Cellar/zeroc-ice33/3.3
            ;;

        ice35 )
        _ICE_HOME=/usr/local/Cellar/ice/3.5.0
            ;;

        # Add other environment settings here

        * )
            echo "echo Unknown environment option: $1"
            exit 1
            ;;
    esac

    if [ -n "$_ICE_HOME" ]; then
        echo export ICE_HOME=\"$_ICE_HOME\"
        echo export PYTHONPATH=\"$_ICE_HOME/python:$PYTHONPATH\"
        echo export PATH=\"$_ICE_HOME/bin:$PATH\"
        echo export DYLD_LIBRARY_PATH=\"$_ICE_HOME/lib:$DYLD_LIBRARY_PATH\"
    fi

    return 0
}

while [ $# -gt 0 ]; do
    eval $(_omero_env "$1")
    echo $?
    shift
done

unset -f _omero_env

@manics
Copy link
Member

manics commented Aug 15, 2013

The other main things to test are whether prefixed ports are correctly handled i.e. stopping the existing server and starting the new one. We should probably define what behaviour we want when these are different. If it's an upgrade should we ignore ~/config.xml? If prefix 1 was specifid for the old server but --prefix 2 for the upgraded do we change the ports? (omero admin ports only works when the config files contains the default ports). If someone wants to revert from --prefix 1 to no prefix is that possible?

@manics
Copy link
Member

manics commented Aug 15, 2013

And one more... what happens if the upgrade fails part way? Is the logic sufficient to handle this situation e.g. do we change the symlink before or after the server is started.

@joshmoore
Copy link
Member Author

With @manic's PR merged, a fresh install now fails with:

shutil.Error: `OMERO-CURRENT/etc/grid/config.xml` and `OMERO.server-5.0.0-beta1-140-5abc4d0-ice34-b3495/etc/grid/config.xml` are the same file

Looking into this.

 * Only use python 2.7 since Ice isn't compiled for 2.6
 * Create a PG user and db "omero"
 * Create /OMERO as $USER
@joshmoore
Copy link
Member Author

Current issue is that no DB is created by omego upgrade. I'll likely need to add a --init-db option to make that work. If upgrade is taking on more of the install actions, should we rename it to server?

@sbesson
Copy link
Member

sbesson commented Aug 16, 2013

I think omego upgrade --init-db is a slightly confusing (especially if we plan to support omego upgrade --purge-db). I would rather go for a omega init-server --with-db or equivalent?

@manics
Copy link
Member

manics commented Aug 16, 2013

+1 to a seperate init/setup/init-server command- maybe this is a better place for the --cfg confg.xml argument. How do you feel about moving all the omero config modifying options to init, for instance is it reasonable for someone to want to change the ports during an upgrade?

@joshmoore
Copy link
Member Author

Seems like if you were going to change the ports during an upgrade, you'd 1) upgrade and then 2) change the ports. So, I agree. Should I go ahead and try to do that work now so we can get the test passing? (The ever lasting PR take 2...)

@manics
Copy link
Member

manics commented Aug 16, 2013

Depends if you have time :) Alternative is to just merge (so that we're back in the original position) and have a new PR for the init command. Are you thinking of having arguments to specify the main options (db.user/db.pass/db.name/data.store), or relying on config.xml?

Pending a refactoring as outlined in omegh-10,
the test which actually downloads and tries
to startup OMERO is now skipped. Once a db
creation command is available, it can be
turned back on.
@joshmoore
Copy link
Member Author

Ok. Knowing that I'm the limiting factor, I've added a commit to skip the download test to get this green again. If there are no final objects (3...2...) let's merge and be back to where we were. (Some jobs may need modifying, @sbesson)

Then we can start work on setup or whatever, which, I think, should take options for the primary values and create a config.xml in the proper location (e.g. ~/config.xml though change this as necessary).

@manics
Copy link
Member

manics commented Aug 16, 2013

👍

joshmoore added a commit that referenced this pull request Aug 16, 2013
@joshmoore joshmoore merged commit 888e7da into ome:master Aug 16, 2013
@joshmoore joshmoore deleted the omego-migration branch August 16, 2013 15:08
@sbesson sbesson mentioned this pull request Sep 16, 2013
@sbesson sbesson added this to the 0.1.0 milestone Dec 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants