From 891bbad041348d50dc578b0ad67808cdb09e3996 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 8 Oct 2020 15:19:49 -0400 Subject: [PATCH 01/14] Add script + wrapper (cherry picked from commit 2dbfdce295e5cbcb0daddc560853967b8aaa35bd) --- check_model.sh | 12 +++++++++ scripts/check_model.py | 57 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100755 check_model.sh create mode 100644 scripts/check_model.py diff --git a/check_model.sh b/check_model.sh new file mode 100755 index 00000000000..67a2f33720e --- /dev/null +++ b/check_model.sh @@ -0,0 +1,12 @@ +#!/bin/sh + +cd "$(dirname "$0")" + +. ./scripts/common_startup_functions.sh + +setup_python + +python ./scripts/check_model.py "$@" + +[ $? -ne 0 ] && exit 1; +exit 0; diff --git a/scripts/check_model.py b/scripts/check_model.py new file mode 100644 index 00000000000..7ca861059d1 --- /dev/null +++ b/scripts/check_model.py @@ -0,0 +1,57 @@ +""" +Check for db indexes defined in mapping.py but missing in the database. +Note: pass '-c /path/to/galaxy.yml' to use the database_connection set in galaxy.yml. +Otherwise the default sqlite database will be used. +""" +import json +import os +import sys +from collections import namedtuple + +sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'lib'))) + +from sqlalchemy import create_engine, MetaData + +from galaxy.model import mapping +from galaxy.model.orm.scripts import get_config + +Index = namedtuple('MissingIndex', 'table column_names') + + +def tuple_from_index(index): + columns = tuple([getattr(index.columns, c).name for c in dir(index.columns) if not c.startswith('__')]) + if len(columns) == 1: + columns = columns[0] + return Index(index.table.name, columns) + + +def find_missing_indexes(): + + def load_indexes(metadata): + indexes = {} + for t in metadata.tables.values(): + for index in t.indexes: + missing_index = tuple_from_index(index) + indexes[missing_index] = index.name + return indexes + + # load metadata from mapping.py + metadata = mapping.metadata + mapping_indexes = load_indexes(metadata) + + # create EMPTY metadata, then load from database + db_url = get_config(sys.argv)['db_url'] + metadata = MetaData(bind=create_engine(db_url)) + metadata.reflect() + indexes_in_db = load_indexes(metadata) + + missing_indexes = set(mapping_indexes.keys()) - set(indexes_in_db.keys()) + if missing_indexes: + return [(mapping_indexes[index], index.table, index.column_names) for index in missing_indexes] + + +if __name__ == '__main__': + indexes = find_missing_indexes() + if indexes: + print(json.dumps(indexes, indent=4)) + sys.exit(1) From 9b421aa8c8ed05c5eab36254de2660fd33dd7975 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 8 Oct 2020 15:20:09 -0400 Subject: [PATCH 02/14] Update scripts/check_model.py Co-authored-by: Nicola Soranzo (cherry picked from commit 7719f53bca2ec21344ba32e9497e5d392f705eb5) --- scripts/check_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check_model.py b/scripts/check_model.py index 7ca861059d1..474b8626643 100644 --- a/scripts/check_model.py +++ b/scripts/check_model.py @@ -53,5 +53,5 @@ def find_missing_indexes(): if __name__ == '__main__': indexes = find_missing_indexes() if indexes: - print(json.dumps(indexes, indent=4)) + print(json.dumps(indexes, indent=4, sort_keys=True)) sys.exit(1) From 6e6e3aa757433d15a592fcb984d682b70f6e24bb Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 1 Oct 2020 22:19:10 -0400 Subject: [PATCH 03/14] Update scripts/check_model.py Co-authored-by: Nicola Soranzo (cherry picked from commit b9093e33dad7f05ede8c3fb5aac65afb41a46d5a) --- scripts/check_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check_model.py b/scripts/check_model.py index 474b8626643..10099e4f0b7 100644 --- a/scripts/check_model.py +++ b/scripts/check_model.py @@ -15,7 +15,7 @@ from sqlalchemy import create_engine, MetaData from galaxy.model import mapping from galaxy.model.orm.scripts import get_config -Index = namedtuple('MissingIndex', 'table column_names') +IndexTuple = namedtuple('IndexTuple', 'table column_names') def tuple_from_index(index): From eb89c1f4109c705e47ad04ba5e9c5dd25d6d6e5d Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 1 Oct 2020 22:22:21 -0400 Subject: [PATCH 04/14] Rename missing_index > index_tuple (cherry picked from commit 20c7cdb2cb15535ea3f86c4609e2caa6466bee65) --- scripts/check_model.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/check_model.py b/scripts/check_model.py index 10099e4f0b7..65e6d7cb4f4 100644 --- a/scripts/check_model.py +++ b/scripts/check_model.py @@ -22,7 +22,7 @@ def tuple_from_index(index): columns = tuple([getattr(index.columns, c).name for c in dir(index.columns) if not c.startswith('__')]) if len(columns) == 1: columns = columns[0] - return Index(index.table.name, columns) + return IndexTuple(index.table.name, columns) def find_missing_indexes(): @@ -31,8 +31,8 @@ def find_missing_indexes(): indexes = {} for t in metadata.tables.values(): for index in t.indexes: - missing_index = tuple_from_index(index) - indexes[missing_index] = index.name + index_tuple = tuple_from_index(index) + indexes[index_tuple] = index.name return indexes # load metadata from mapping.py From 7d75f313cff3e4b65cb59b6bd247a13f98096727 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Fri, 2 Oct 2020 10:54:29 -0400 Subject: [PATCH 05/14] Add environment variable that forces migrations on empty database (cherry picked from commit bd8bfdfec68c7074415aade49e762f7b08bc8911) --- lib/galaxy/model/migrate/check.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/model/migrate/check.py b/lib/galaxy/model/migrate/check.py index afea334af03..70036dc4c1c 100644 --- a/lib/galaxy/model/migrate/check.py +++ b/lib/galaxy/model/migrate/check.py @@ -62,12 +62,13 @@ def create_or_verify_database(url, galaxy_config_file, engine_options={}, app=No migrate_to_current_version(engine, db_schema) def migrate_from_scratch(): - log.info("Creating new database from scratch, skipping migrations") - current_version = migrate_repository.version().version - mapping.init(file_path='/tmp', url=url, map_install_models=map_install_models, create_tables=True) - schema.ControlledSchema.create(engine, migrate_repository, version=current_version) - db_schema = schema.ControlledSchema(engine, migrate_repository) - assert db_schema.version == current_version + if not os.environ.get("GALAXY_TEST_FORCE_DATABASE_MIGRATION"): + log.info("Creating new database from scratch, skipping migrations") + current_version = migrate_repository.version().version + mapping.init(file_path='/tmp', url=url, map_install_models=map_install_models, create_tables=True) + schema.ControlledSchema.create(engine, migrate_repository, version=current_version) + db_schema = schema.ControlledSchema(engine, migrate_repository) + assert db_schema.version == current_version migrate() if app: # skips the tool migration process. From 08ad0f47c1b7671ca0f9f66782fd978c625d15b4 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Fri, 2 Oct 2020 11:14:53 -0400 Subject: [PATCH 06/14] Add script to tox.ini (cherry picked from commit f11baa411c09ad67ec2f2ea93e254e1623ad9f89) --- tox.ini | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tox.ini b/tox.ini index f978eea07b6..ac963d46f1e 100644 --- a/tox.ini +++ b/tox.ini @@ -21,6 +21,7 @@ setenv = unit: GALAXY_VIRTUAL_ENV={envdir} unit: GALAXY_ENABLE_BETA_COMPRESSED_GENBANK_SNIFFING=1 mulled: GALAXY_TEST_INCLUDE_SLOW=1 + check_indexes: GALAXY_TEST_FORCE_DATABASE_MIGRATION=1 deps = lint,lint_docstring,lint_docstring_include_list: -rlib/galaxy/dependencies/pipfiles/flake8/pinned-requirements.txt unit: mock @@ -47,3 +48,8 @@ commands = bash .ci/validate_test_tools.sh [testenv:web_controller_line_count] commands = bash .ci/check_controller.sh + +[testenv:check_indexes] +commands = + bash create_db.sh + bash check_model.sh From af0f9bb6ca9e8a5c928baffd1531c1f1a5b740c3 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Fri, 2 Oct 2020 22:41:42 -0400 Subject: [PATCH 07/14] Add github workflow; env var to override db uri (cherry picked from commit f27b4e50f665ab32737d79294e5ca7c7ce48c476) --- .github/workflows/db_indexes.yaml | 45 +++++++++++++++++++++++++++++++ scripts/check_model.py | 2 +- 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/db_indexes.yaml diff --git a/.github/workflows/db_indexes.yaml b/.github/workflows/db_indexes.yaml new file mode 100644 index 00000000000..846f2cc5956 --- /dev/null +++ b/.github/workflows/db_indexes.yaml @@ -0,0 +1,45 @@ +name: Database indexes +on: [push, pull_request] +defaults: + run: + working-directory: 'galaxy root' +jobs: + check: + name: Check database indexes + runs-on: ubuntu-latest + strategy: + matrix: + python-version: [3.7] + db: ['postgresql', 'sqlite'] + services: + postgres: + image: postgres:11 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: postgres + ports: + - 5432:5432 + steps: + - uses: actions/checkout@v2 + with: + path: 'galaxy root' + - uses: actions/setup-python@v1 + with: + python-version: ${{ matrix.python-version }} + - name: Cache pip dir + uses: actions/cache@v1 + id: pip-cache + with: + path: ~/.cache/pip + key: pip-cache-${{ matrix.python-version }}-${{ hashFiles('galaxy root/requirements.txt') }} + - name: Install tox + run: pip install tox + - name: Check indexes on postgresql + if: matrix.db == 'postgresql' + env: + GALAXY_CONFIG_OVERRIDE_DATABASE_CONNECTION: 'postgres://postgres:postgres@localhost:5432/galaxy?client_encoding=utf8' + run: tox -e check_indexes + - name: Check indexes on sqlite + if: matrix.db == 'sqlite' + run: tox -e check_indexes diff --git a/scripts/check_model.py b/scripts/check_model.py index 65e6d7cb4f4..b8ef7dfbe01 100644 --- a/scripts/check_model.py +++ b/scripts/check_model.py @@ -40,7 +40,7 @@ def find_missing_indexes(): mapping_indexes = load_indexes(metadata) # create EMPTY metadata, then load from database - db_url = get_config(sys.argv)['db_url'] + db_url = os.environ.get("GALAXY_CONFIG_OVERRIDE_DATABASE_CONNECTION") or get_config(sys.argv)['db_url'] metadata = MetaData(bind=create_engine(db_url)) metadata.reflect() indexes_in_db = load_indexes(metadata) From de98e1f64ed96c4d10aae12bf130e0e0e568f930 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sat, 3 Oct 2020 02:27:47 -0400 Subject: [PATCH 08/14] Call common_startup.sh; rebase (cherry picked from commit 2cc090847e7232bfa0f8140dc4810a0cae452665) --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index ac963d46f1e..b69972001ad 100644 --- a/tox.ini +++ b/tox.ini @@ -51,5 +51,6 @@ commands = bash .ci/check_controller.sh [testenv:check_indexes] commands = + bash scripts/common_startup.sh bash create_db.sh bash check_model.sh From b98677cd9a02d239564fb3a1ea319267421557a1 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 8 Oct 2020 15:21:47 -0400 Subject: [PATCH 09/14] Skip client build Co-authored-by: Marius van den Beek (cherry picked from commit 7040c0056b8a8bd9efb765c32f3ef761de5852c3) --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index b69972001ad..0bff7883189 100644 --- a/tox.ini +++ b/tox.ini @@ -22,6 +22,7 @@ setenv = unit: GALAXY_ENABLE_BETA_COMPRESSED_GENBANK_SNIFFING=1 mulled: GALAXY_TEST_INCLUDE_SLOW=1 check_indexes: GALAXY_TEST_FORCE_DATABASE_MIGRATION=1 + check_indexes: GALAXY_SKIP_CLIENT_BUILD=1 deps = lint,lint_docstring,lint_docstring_include_list: -rlib/galaxy/dependencies/pipfiles/flake8/pinned-requirements.txt unit: mock From 84c3967ce87964cf3a8f88c1ec0ec54554fff04c Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 8 Oct 2020 15:22:38 -0400 Subject: [PATCH 10/14] Use passenv to pass env var to tox (cherry picked from commit 65ebbce4582719df7e6906578e4ccd3aaaa5361c) --- tox.ini | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 0bff7883189..fe08a8cd78c 100644 --- a/tox.ini +++ b/tox.ini @@ -13,7 +13,9 @@ commands = unit: bash run_tests.sh -u whitelist_externals = bash -passenv = CI CONDA_EXE +passenv = + CI CONDA_EXE + GALAXY_CONFIG_OVERRIDE_DATABASE_CONNECTION setenv = first_startup: GALAXY_PYTHON=python first_startup: GALAXY_CONFIG_DATABASE_AUTO_MIGRATE=true From d3edae0012319dffb4ebac37a3ea165a8e05d91c Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 8 Oct 2020 15:23:19 -0400 Subject: [PATCH 11/14] Let get_config() handle env override Co-authored-by: Marius van den Beek (cherry picked from commit 574fa4cb1f3f72537b6ac75205d260b97714dc6d) --- scripts/check_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check_model.py b/scripts/check_model.py index b8ef7dfbe01..65e6d7cb4f4 100644 --- a/scripts/check_model.py +++ b/scripts/check_model.py @@ -40,7 +40,7 @@ def find_missing_indexes(): mapping_indexes = load_indexes(metadata) # create EMPTY metadata, then load from database - db_url = os.environ.get("GALAXY_CONFIG_OVERRIDE_DATABASE_CONNECTION") or get_config(sys.argv)['db_url'] + db_url = get_config(sys.argv)['db_url'] metadata = MetaData(bind=create_engine(db_url)) metadata.reflect() indexes_in_db = load_indexes(metadata) From d6ceb9cc9f9105ed4ca1c7164c252276a921fb46 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 8 Oct 2020 15:23:47 -0400 Subject: [PATCH 12/14] Work around sqlite/foreign key/index bug (cherry picked from commit 8f8e25a1b7a43e9247a99e31d2cfa2991ac4704a) --- lib/galaxy/model/migrate/versions/util.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/galaxy/model/migrate/versions/util.py b/lib/galaxy/model/migrate/versions/util.py index feb36210150..019b78de297 100644 --- a/lib/galaxy/model/migrate/versions/util.py +++ b/lib/galaxy/model/migrate/versions/util.py @@ -86,6 +86,7 @@ def add_column(column, table, metadata, **kwds): :type metadata: :class:`Metadata` """ try: + index_to_create = None migrate_engine = metadata.bind if not isinstance(table, Table): table = Table(table, metadata, autoload=True) @@ -93,10 +94,14 @@ def add_column(column, table, metadata, **kwds): # SQLAlchemy Migrate has a bug when adding a column with both a # ForeignKey and an index in SQLite. Since SQLite creates an index # anyway, we can drop the explicit index creation. + # TODO: this is hacky, but it solves this^ problem. Needs better solution. + index_to_create = (kwds['index_name'], table, column.name) del kwds['index_name'] column.index = False column.create(table, **kwds) assert column is table.c[column.name] + if index_to_create: + add_index(*index_to_create) except Exception: log.exception("Adding column '%s' to table '%s' failed.", column, table) From e4156ef10e46856fc5b99ea7d3b8babf6f5157c7 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 8 Oct 2020 15:23:59 -0400 Subject: [PATCH 13/14] Add more missing indexes (cherry picked from commit 327adb62d60c496f148ccbfdca12f67d81ae33f7) --- lib/galaxy/model/mapping.py | 10 ++-- .../versions/0170_add_more_missing_indexes.py | 60 +++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 lib/galaxy/model/migrate/versions/0170_add_more_missing_indexes.py diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py index 11e84951eca..7d68dd3a8d4 100644 --- a/lib/galaxy/model/mapping.py +++ b/lib/galaxy/model/mapping.py @@ -1125,18 +1125,18 @@ model.WorkflowInvocationOutputDatasetAssociation.table = Table( "workflow_invocation_output_dataset_association", metadata, Column("id", Integer, primary_key=True), Column("workflow_invocation_id", Integer, ForeignKey("workflow_invocation.id"), index=True), - Column("workflow_step_id", Integer, ForeignKey("workflow_step.id")), + Column("workflow_step_id", Integer, ForeignKey("workflow_step.id"), index=True), Column("dataset_id", Integer, ForeignKey("history_dataset_association.id"), index=True), - Column("workflow_output_id", Integer, ForeignKey("workflow_output.id")), + Column("workflow_output_id", Integer, ForeignKey("workflow_output.id"), index=True), ) model.WorkflowInvocationOutputDatasetCollectionAssociation.table = Table( "workflow_invocation_output_dataset_collection_association", metadata, Column("id", Integer, primary_key=True), Column("workflow_invocation_id", Integer, ForeignKey("workflow_invocation.id", name='fk_wiodca_wii'), index=True), - Column("workflow_step_id", Integer, ForeignKey("workflow_step.id", name='fk_wiodca_wsi')), + Column("workflow_step_id", Integer, ForeignKey("workflow_step.id", name='fk_wiodca_wsi'), index=True), Column("dataset_collection_id", Integer, ForeignKey("history_dataset_collection_association.id", name='fk_wiodca_dci'), index=True), - Column("workflow_output_id", Integer, ForeignKey("workflow_output.id", name='fk_wiodca_woi')), + Column("workflow_output_id", Integer, ForeignKey("workflow_output.id", name='fk_wiodca_woi'), index=True), ) model.WorkflowInvocationOutputValue.table = Table( @@ -1160,7 +1160,7 @@ model.WorkflowInvocationStepOutputDatasetCollectionAssociation.table = Table( "workflow_invocation_step_output_dataset_collection_association", metadata, Column("id", Integer, primary_key=True), Column("workflow_invocation_step_id", Integer, ForeignKey("workflow_invocation_step.id", name='fk_wisodca_wisi'), index=True), - Column("workflow_step_id", Integer, ForeignKey("workflow_step.id", name='fk_wisodca_wsi')), + Column("workflow_step_id", Integer, ForeignKey("workflow_step.id", name='fk_wisodca_wsi'), index=True), Column("dataset_collection_id", Integer, ForeignKey("history_dataset_collection_association.id", name='fk_wisodca_dci'), index=True), Column("output_name", String(255), nullable=True), ) diff --git a/lib/galaxy/model/migrate/versions/0170_add_more_missing_indexes.py b/lib/galaxy/model/migrate/versions/0170_add_more_missing_indexes.py new file mode 100644 index 00000000000..d5f5fa54703 --- /dev/null +++ b/lib/galaxy/model/migrate/versions/0170_add_more_missing_indexes.py @@ -0,0 +1,60 @@ +""" +Migration script to create missing indexes. Adding new columns to existing tables via SQLAlchemy does not create the index, even if the column definition includes index=True. +""" + +import logging + +from sqlalchemy import MetaData + +from galaxy.model.migrate.versions.util import ( + add_index, + drop_index +) + +log = logging.getLogger(__name__) +metadata = MetaData() + +indexes = [ + [ + "ix_workflow_invocation_output_dataset_association_workflow_output_id", + "workflow_invocation_output_dataset_association", + "workflow_output_id" + ], + [ + "ix_workflow_invocation_output_dataset_association_workflow_step_id", + "workflow_invocation_output_dataset_association", + "workflow_step_id" + ], + [ + "ix_workflow_invocation_output_dataset_collection_association_workflow_output_id", + "workflow_invocation_output_dataset_collection_association", + "workflow_output_id" + ], + [ + "ix_workflow_invocation_output_dataset_collection_association_workflow_step_id", + "workflow_invocation_output_dataset_collection_association", + "workflow_step_id" + ], + [ + "ix_workflow_invocation_step_output_dataset_collection_association_workflow_step_id", + "workflow_invocation_step_output_dataset_collection_association", + "workflow_step_id" + ], +] + + +def upgrade(migrate_engine): + print(__doc__) + metadata.bind = migrate_engine + metadata.reflect() + + for ix, table, col in indexes: + add_index(ix, table, col, metadata) + + +def downgrade(migrate_engine): + metadata.bind = migrate_engine + metadata.reflect() + + for ix, table, col in indexes: + drop_index(ix, table, col, metadata) From 715b370fc9e9969628326dad21b293aa090d5430 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 9 Oct 2020 11:08:41 +0200 Subject: [PATCH 14/14] Fix regex replacement wrongly appearing in rule builder Fixes https://github.com/galaxyproject/galaxy/issues/10389 --- client/galaxy/scripts/components/RuleCollectionBuilder.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/galaxy/scripts/components/RuleCollectionBuilder.vue b/client/galaxy/scripts/components/RuleCollectionBuilder.vue index bba1152fc17..23210f98975 100644 --- a/client/galaxy/scripts/components/RuleCollectionBuilder.vue +++ b/client/galaxy/scripts/components/RuleCollectionBuilder.vue @@ -1962,7 +1962,7 @@ export default { this.addColumnRegexGroupCount = 1; } if (val == "replacement") { - this.addColumnRegexReplacement = "\0"; + this.addColumnRegexReplacement = null; } } },