Skip to content

Sql schema files refactoring#92

Merged
manics merged 4 commits intoome:masterfrom
sbesson:sql_schema_files
Jun 29, 2016
Merged

Sql schema files refactoring#92
manics merged 4 commits intoome:masterfrom
sbesson:sql_schema_files

Conversation

@sbesson
Copy link
Member

@sbesson sbesson commented Jun 28, 2016

Based the discussion in ome/openmicroscopy#4732, this PR reviews and reviews the way omego parses and sorts SQL schema files to compute a database upgrade graph.

DB upgrade strategy

While upgrading a version of the server, in order to determine whether the current database needs to be upgraded, omego uses the following workflow:

  • DbAdmin.get_current_db_version() retrieve the current DB schema
  • DbAdmin.sql_version_matrix() retrieves all DB schemas available from the sql/psql folders and creates a transformation matrix
  • DbAdmin.sql_version_resolve() resolves the upgrade path given a source and a target DB schema

SQL file classification

At the moment, omego.db module classifies SQL files found on the OMERO server into two categories:

  • SQL schema files used for computing a DB upgrade path and defined via a regular expression, typically OMEROM.m__p
  • other SQL files which can include header, footer but also optional SQL files

Changes

This Pull Request reviews the way DbAdmin.sql_version_matrix() works to handle non-DB schema files in a more robust manner. The summary of changes is the following:

  • exposes the regular expression defining DB schema files at the module level
  • adds module level functions to validate a DB schema is_schema() and sort a list of schemas sort_schemas()
  • refactors the parsing of a set of SQL files as parse_schema_files() which uses is_schema() internally and constructs a dictionary of valid DB schema files with their associated source and target schemas
  • rewrites DbAdmin.sql_version_matrix() to drop the internal behavior and special case handling and delegate to parse_schema_files() instead

Testing

Unit tests have been added to test/unit/test_db.py to cover the semantics of the new functions especially the DB schema validation and parsing functionalities. All tests should pass on Travis.

As an integration test, performing a dry-run upgrade of the DB using the following command against a server including ome/openmicroscopy#4732 should now return instead of failing:

$ python omego/main.py db upgrade -n --serverdir /path/to/serverdir

Extension points

This PR does not cover improvements to parse precheck SQL scripts and run them prior to the DB upgrade. Keeping this use case in mind, an idea might be to define a dictionary of regular expressions and pass a parameter to the top-level methods to specify the type of SQL file to validate, sort and parse.

sbesson added 4 commits June 28, 2016 13:47
Update unit tests to not depend on the mock logic
- remove hard-coded Windows handling
- validate every sql source/target using the is_schema()
- builds a dictionary of valid schema files
log = logging.getLogger("omego.db")

# Regular expression identifying a SQL schema
SQL_SCHEMA_REGEXP = re.compile('.*OMERO(\d+)(\.|A)?(\d*)([A-Z]*)__(\d+)$')
Copy link
Member

Choose a reason for hiding this comment

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

I'd replace OMERO here with [a-zA-Z]

Copy link
Member

Choose a reason for hiding this comment

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

How should those sql files be handled?

Copy link
Member

Choose a reason for hiding this comment

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

"OMERO" is not special in the name. It's just an arbitrary identifier. It's valid to have a sequence like:

OMERO5.2__0 --> IDR1__0 --> IDR__2__0 --> OMERO5.3__0

and in fact, we may soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can either include this change as part of this PR of a follow-up one (since this PR tries to solve an immediate deployment issue). One question here is about the schemas ordering logic i.e. what are the rules to compute the order listed above (we have one more discriminator tahn the previous M.m__p)?

Copy link
Member

Choose a reason for hiding this comment

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

👍 for a separate PR

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yup, not this PR's fault.

@manics
Copy link
Member

manics commented Jun 29, 2016

Dry-run with latest release

$ venv/bin/omego db upgrade --serverdir OMERO.server-5.2.4-ice35-b23 --dbname omero -nv
...
2016-06-29 16:26:11,834 [    omego.db] DEBUG stdout: OMERO4.3|0

2016-06-29 16:26:11,834 [    omego.db] INFO  Current omero db version: ('OMERO4.3', '0')
2016-06-29 16:26:11,836 [    omego.db] INFO  Upgrading database using OMERO.server-5.2.4-ice35-b23/sql/psql/OMERO4.4__0/OMERO4.3__0.sql
2016-06-29 16:26:11,836 [    omego.db] INFO  Upgrading database using OMERO.server-5.2.4-ice35-b23/sql/psql/OMERO5.0__0/OMERO4.4__0.sql
2016-06-29 16:26:11,836 [    omego.db] INFO  Upgrading database using OMERO.server-5.2.4-ice35-b23/sql/psql/OMERO5.1__1/OMERO5.0__0.sql
2016-06-29 16:26:11,836 [    omego.db] INFO  Upgrading database using OMERO.server-5.2.4-ice35-b23/sql/psql/OMERO5.2__0/OMERO5.1__1.sql
2016-06-29 16:26:11,837 [omero.util.T] DEBUG Removing tree: /home/build/omero/tmp/omero_build/699

Dry-run with OMERO-DEV-merge-build with manual patch from https://patch-diff.githubusercontent.com/raw/openmicroscopy/openmicroscopy/pull/4732.diff

$ ls OMERO.server-5.3.0-m2-730-e8279e2-ice35-b355/sql/psql/OMERO5.3DEV__7
delete-ns-orphans.sql     OMERO5.2__0.sql     psql-header.sql
enums_update.sql          OMERO5.3DEV__6.sql  schema.sql
OMERO5.2__0-precheck.sql  psql-footer.sql     views.sql

$ venv/bin/omego db upgrade --serverdir OMERO.server-5.3.0-m2-730-e8279e2-ice35-b355 --dbname omero -nv
...
2016-06-29 16:32:00,949 [    omego.db] DEBUG stdout: OMERO4.3|0

2016-06-29 16:32:00,949 [    omego.db] INFO  Current omero db version: ('OMERO4.3', '0')
2016-06-29 16:32:00,951 [    omego.db] INFO  Upgrading database using OMERO.server-5.3.0-m2-730-e8279e2-ice35-b355/sql/psql/OMERO4.4__0/OMERO4.3__0.sql
2016-06-29 16:32:00,951 [    omego.db] INFO  Upgrading database using OMERO.server-5.3.0-m2-730-e8279e2-ice35-b355/sql/psql/OMERO5.0__0/OMERO4.4__0.sql
2016-06-29 16:32:00,951 [    omego.db] INFO  Upgrading database using OMERO.server-5.3.0-m2-730-e8279e2-ice35-b355/sql/psql/OMERO5.1__1/OMERO5.0__0.sql
2016-06-29 16:32:00,951 [    omego.db] INFO  Upgrading database using OMERO.server-5.3.0-m2-730-e8279e2-ice35-b355/sql/psql/OMERO5.2__0/OMERO5.1__1.sql
2016-06-29 16:32:00,951 [    omego.db] INFO  Upgrading database using OMERO.server-5.3.0-m2-730-e8279e2-ice35-b355/sql/psql/OMERO5.3DEV__7/OMERO5.2__0.sql
2016-06-29 16:32:00,952 [omero.util.T] DEBUG Removing tree: /home/build/omero/tmp/omero_build/736

Same, but without -n:

...
YOU HAVE SUCCESSFULLY UPGRADED YOUR DATABASE TO VERSION OMERO5.3DEV__7
...
$ psql -h localhost -Uomero
Password for user omero:
psql (9.4.8)
Type "help" for help.

omero=> select * from dbpatch;
 id | currentpatch | currentversion | permissions |          finished          |
      message      | previouspatch | previousversion | external_id
----+--------------+----------------+-------------+----------------------------+
-------------------+---------------+-----------------+-------------
  1 |            0 | OMERO4.3       |         -35 | 2016-06-29 16:11:19.601502 |
 Database ready.   |             0 | OMERO4.3        |
  2 |            0 | OMERO4.4       |         -35 | 2016-06-29 16:32:45.536261 |
 Database updated. |             0 | OMERO4.3        |
  3 |            0 | OMERO5.0       |         -35 | 2016-06-29 16:32:45.6669   |
 Database updated. |             0 | OMERO4.4        |
  4 |            1 | OMERO5.1       |         -35 | 2016-06-29 16:32:46.445101 |
 Database updated. |             0 | OMERO5.0        |
  5 |            0 | OMERO5.2       |         -35 | 2016-06-29 16:32:46.506875 |
 Database updated. |             1 | OMERO5.1        |
  6 |            7 | OMERO5.3DEV    |         -35 | 2016-06-29 16:32:46.682694 |
 Database updated. |             0 | OMERO5.2        |
(6 rows)

@manics manics merged commit 18fd7bd into ome:master Jun 29, 2016
@sbesson sbesson deleted the sql_schema_files branch June 29, 2016 17:40
@sbesson sbesson added this to the 0.4.1 milestone Jun 29, 2016
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.

3 participants