From 2dbfdce295e5cbcb0daddc560853967b8aaa35bd Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 1 Oct 2020 20:23:40 -0400 Subject: [PATCH 01/16] Add script + wrapper --- 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 7719f53bca2ec21344ba32e9497e5d392f705eb5 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 1 Oct 2020 22:03:41 -0400 Subject: [PATCH 02/16] Update scripts/check_model.py Co-authored-by: Nicola Soranzo --- 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 b9093e33dad7f05ede8c3fb5aac65afb41a46d5a Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 1 Oct 2020 22:19:10 -0400 Subject: [PATCH 03/16] Update scripts/check_model.py Co-authored-by: Nicola Soranzo --- 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 20c7cdb2cb15535ea3f86c4609e2caa6466bee65 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Thu, 1 Oct 2020 22:22:21 -0400 Subject: [PATCH 04/16] Rename missing_index > index_tuple --- 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 bd8bfdfec68c7074415aade49e762f7b08bc8911 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Fri, 2 Oct 2020 10:54:29 -0400 Subject: [PATCH 05/16] Add environment variable that forces migrations on empty database --- 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 f11baa411c09ad67ec2f2ea93e254e1623ad9f89 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Fri, 2 Oct 2020 11:14:53 -0400 Subject: [PATCH 06/16] Add script to tox.ini --- 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 f27b4e50f665ab32737d79294e5ca7c7ce48c476 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Fri, 2 Oct 2020 22:41:42 -0400 Subject: [PATCH 07/16] Add github workflow; env var to override db uri --- .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 2cc090847e7232bfa0f8140dc4810a0cae452665 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sat, 3 Oct 2020 02:27:47 -0400 Subject: [PATCH 08/16] Call common_startup.sh; rebase --- 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 92add099f60ccce595f65e1da4b86b0ca06e05f5 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 1 Oct 2020 20:17:36 +0200 Subject: [PATCH 09/16] Drop unused models --- lib/galaxy/model/__init__.py | 17 ----------------- lib/galaxy/model/mapping.py | 24 ------------------------ 2 files changed, 41 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 2b10f722984..8b6fd220988 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -3909,19 +3909,6 @@ class LibraryDatasetDatasetInfoAssociation(RepresentById): return True # always allow inheriting, used for replacement -class ValidationError(RepresentById): - def __init__(self, message=None, err_type=None, attributes=None): - self.message = message - self.err_type = err_type - self.attributes = attributes - - -class DatasetToValidationErrorAssociation: - def __init__(self, dataset, validation_error): - self.dataset = dataset - self.validation_error = validation_error - - class ImplicitlyConvertedDatasetAssociation(RepresentById): def __init__(self, id=None, parent=None, dataset=None, file_type=None, deleted=False, purged=False, metadata_safe=True): @@ -6170,10 +6157,6 @@ class HistoryTagAssociation(ItemTagAssociation, RepresentById): pass -class DatasetTagAssociation(ItemTagAssociation, RepresentById): - pass - - class HistoryDatasetAssociationTagAssociation(ItemTagAssociation, RepresentById): pass diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py index d6dc167fcd1..11e84951eca 100644 --- a/lib/galaxy/model/mapping.py +++ b/lib/galaxy/model/mapping.py @@ -333,14 +333,6 @@ model.ImplicitlyConvertedDatasetAssociation.table = Table( Column("metadata_safe", Boolean, index=True, default=True), Column("type", TrimmedString(255))) -model.ValidationError.table = Table( - "validation_error", metadata, - Column("id", Integer, primary_key=True), - Column("dataset_id", Integer, ForeignKey("history_dataset_association.id"), index=True), - Column("message", TrimmedString(255)), - Column("err_type", TrimmedString(64)), - Column("attributes", TEXT)) - model.Group.table = Table( "galaxy_group", metadata, Column("id", Integer, primary_key=True), @@ -1342,16 +1334,6 @@ model.HistoryTagAssociation.table = Table( Column("value", TrimmedString(255), index=True), Column("user_value", TrimmedString(255), index=True)) -model.DatasetTagAssociation.table = Table( - "dataset_tag_association", metadata, - Column("id", Integer, primary_key=True), - Column("dataset_id", Integer, ForeignKey("dataset.id"), index=True), - Column("tag_id", Integer, ForeignKey("tag.id"), index=True), - Column("user_id", Integer, ForeignKey("galaxy_user.id"), index=True), - Column("user_tname", TrimmedString(255), index=True), - Column("value", TrimmedString(255), index=True), - Column("user_value", TrimmedString(255), index=True)) - model.HistoryDatasetAssociationTagAssociation.table = Table( "history_dataset_association_tag_association", metadata, Column("id", Integer, primary_key=True), @@ -1720,8 +1702,6 @@ mapper(model.CloudAuthz, model.CloudAuthz.table, properties=dict( backref='cloudauthz') )) -mapper(model.ValidationError, model.ValidationError.table) - simple_mapping(model.DynamicTool) simple_mapping(model.HistoryDatasetAssociation, @@ -1785,9 +1765,6 @@ simple_mapping(model.Dataset, primaryjoin=( (model.Dataset.table.c.id == model.LibraryDatasetDatasetAssociation.table.c.dataset_id) & (model.LibraryDatasetDatasetAssociation.table.c.deleted == false()))), - tags=relation(model.DatasetTagAssociation, - order_by=model.DatasetTagAssociation.table.c.id, - backref='datasets') ) mapper(model.DatasetHash, model.DatasetHash.table, properties=dict( @@ -2752,7 +2729,6 @@ def tag_mapping(tag_association_class, backref_name): tag_mapping(model.HistoryTagAssociation, "tagged_histories") -tag_mapping(model.DatasetTagAssociation, "tagged_datasets") tag_mapping(model.HistoryDatasetAssociationTagAssociation, "tagged_history_dataset_associations") tag_mapping(model.LibraryDatasetDatasetAssociationTagAssociation, "tagged_library_dataset_dataset_associations") tag_mapping(model.PageTagAssociation, "tagged_pages") From 0e00e9e60dac3e0ef2158a42f58fe151d4d8a448 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 1 Oct 2020 20:20:05 +0200 Subject: [PATCH 10/16] Add missing indexes --- .../versions/0169_add_missing_indexes.py | 130 ++++++++++++++++++ lib/galaxy/model/migrate/versions/util.py | 14 ++ 2 files changed, 144 insertions(+) create mode 100644 lib/galaxy/model/migrate/versions/0169_add_missing_indexes.py diff --git a/lib/galaxy/model/migrate/versions/0169_add_missing_indexes.py b/lib/galaxy/model/migrate/versions/0169_add_missing_indexes.py new file mode 100644 index 00000000000..a9b56294e59 --- /dev/null +++ b/lib/galaxy/model/migrate/versions/0169_add_missing_indexes.py @@ -0,0 +1,130 @@ +""" +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_galaxy_user_activation_token", + "galaxy_user", + "activation_token" + ], + [ + "ix_workflow_step_dynamic_tool_id", + "workflow_step", + "dynamic_tool_id" + ], + [ + "ix_history_dataset_association_version", + "history_dataset_association", + "version" + ], + [ + "ix_workflow_invocation_scheduler", + "workflow_invocation", + "scheduler" + ], + [ + "ix_page_slug", + "page", + "slug" + ], + [ + "ix_workflow_invocation_state", + "workflow_invocation", + "state" + ], + [ + "ix_history_dataset_collection_association_implicit_collection_jobs_id", + "history_dataset_collection_association", + "implicit_collection_jobs_id" + ], + [ + "ix_workflow_step_subworkflow_id", + "workflow_step", + "subworkflow_id" + ], + [ + "ix_dynamic_tool_update_time", + "dynamic_tool", + "update_time" + ], + [ + "ix_library_dataset_dataset_association_extended_metadata_id", + "library_dataset_dataset_association", + "extended_metadata_id" + ], + [ + "ix_workflow_invocation_step_implicit_collection_jobs_id", + "workflow_invocation_step", + "implicit_collection_jobs_id" + ], + [ + "ix_workflow_invocation_step_state", + "workflow_invocation_step", + "state" + ], + [ + "ix_workflow_invocation_history_id", + "workflow_invocation", + "history_id" + ], + [ + "ix_workflow_parent_workflow_id", + "workflow", + "parent_workflow_id" + ], + [ + "ix_metadata_file_uuid", + "metadata_file", + "uuid" + ], + [ + "ix_history_dataset_collection_association_job_id", + "history_dataset_collection_association", + "job_id" + ], + [ + "ix_galaxy_user_active", + "galaxy_user", + "active" + ], + [ + "ix_job_dynamic_tool_id", + "job", + "dynamic_tool_id" + ], + [ + "ix_history_dataset_association_extended_metadata_id", + "history_dataset_association", + "extended_metadata_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) diff --git a/lib/galaxy/model/migrate/versions/util.py b/lib/galaxy/model/migrate/versions/util.py index 273939147b2..9b9334ba9e9 100644 --- a/lib/galaxy/model/migrate/versions/util.py +++ b/lib/galaxy/model/migrate/versions/util.py @@ -1,3 +1,4 @@ +import hashlib import logging from sqlalchemy import ( @@ -47,6 +48,14 @@ def localtimestamp(migrate_engine): raise Exception('Unable to convert data for unknown database type: %s' % migrate_engine.name) +def truncate_index_name(index_name, engine): + # does what sqlalchemy does, see https://github.com/sqlalchemy/sqlalchemy/blob/8455a11bcc23e97afe666873cd872b0f204848d8/lib/sqlalchemy/sql/compiler.py#L4696 + max_index_name_length = engine.dialect.max_index_name_length or engine.dialect.max_identifier_length + if len(index_name) > max_index_name_length: + suffix = hashlib.md5(index_name.encode('utf-8')).hexdigest()[-4:] + index_name = "{trunc}_{suffix}".format(trunc=index_name[0 : max_index_name_length - 8], suffix=suffix) + return index_name + def create_table(table): try: table.create() @@ -135,10 +144,14 @@ def add_index(index_name, table, column_name, metadata=None, **kwds): :param metadata: Needed only if ``table`` is a table name :type metadata: :class:`Metadata` """ + if len(index_name) > 63 and metadata.bind.name in ('postgres', 'postgresql'): + suffix = hashlib.md5(index_name.encode('utf-8')).hexdigest()[-4:] + try: if not isinstance(table, Table): assert metadata is not None table = Table(table, metadata, autoload=True) + index_name = truncate_index_name(index_name, table.metadata.bind) if index_name not in [ix.name for ix in table.indexes]: column = table.c[column_name] # MySQL cannot index a TEXT/BLOB column without specifying mysql_length @@ -168,6 +181,7 @@ def drop_index(index, table, column_name=None, metadata=None): if not isinstance(table, Table): assert metadata is not None table = Table(table, metadata, autoload=True) + index = truncate_index_name(index, table.metadata.bind) if index in [ix.name for ix in table.indexes]: index = Index(index, table.c[column_name]) else: From ac9b6b63d0c114fb1a9f03d1d382f057a3a1d34a Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 2 Oct 2020 10:47:19 +0200 Subject: [PATCH 11/16] Cleanup add_index/drop_index --- lib/galaxy/model/migrate/versions/util.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/model/migrate/versions/util.py b/lib/galaxy/model/migrate/versions/util.py index 9b9334ba9e9..feb36210150 100644 --- a/lib/galaxy/model/migrate/versions/util.py +++ b/lib/galaxy/model/migrate/versions/util.py @@ -56,6 +56,7 @@ def truncate_index_name(index_name, engine): index_name = "{trunc}_{suffix}".format(trunc=index_name[0 : max_index_name_length - 8], suffix=suffix) return index_name + def create_table(table): try: table.create() @@ -144,9 +145,6 @@ def add_index(index_name, table, column_name, metadata=None, **kwds): :param metadata: Needed only if ``table`` is a table name :type metadata: :class:`Metadata` """ - if len(index_name) > 63 and metadata.bind.name in ('postgres', 'postgresql'): - suffix = hashlib.md5(index_name.encode('utf-8')).hexdigest()[-4:] - try: if not isinstance(table, Table): assert metadata is not None @@ -181,9 +179,9 @@ def drop_index(index, table, column_name=None, metadata=None): if not isinstance(table, Table): assert metadata is not None table = Table(table, metadata, autoload=True) - index = truncate_index_name(index, table.metadata.bind) - if index in [ix.name for ix in table.indexes]: - index = Index(index, table.c[column_name]) + index_name = truncate_index_name(index, table.metadata.bind) + if index_name in [ix.name for ix in table.indexes]: + index = Index(index_name, table.c[column_name]) else: log.debug("Index '%s' in table '%s' does not exist.", index, table) return From 7040c0056b8a8bd9efb765c32f3ef761de5852c3 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 4 Oct 2020 22:16:32 -0400 Subject: [PATCH 12/16] Skip client build Co-authored-by: Marius van den Beek --- 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 65ebbce4582719df7e6906578e4ccd3aaaa5361c Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 4 Oct 2020 22:48:06 -0400 Subject: [PATCH 13/16] Use passenv to pass env var to tox --- 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 574fa4cb1f3f72537b6ac75205d260b97714dc6d Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 4 Oct 2020 23:25:23 -0400 Subject: [PATCH 14/16] Let get_config() handle env override Co-authored-by: Marius van den Beek --- 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 8f8e25a1b7a43e9247a99e31d2cfa2991ac4704a Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Mon, 5 Oct 2020 01:34:58 -0400 Subject: [PATCH 15/16] Work around sqlite/foreign key/index bug --- 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 327adb62d60c496f148ccbfdca12f67d81ae33f7 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Wed, 7 Oct 2020 11:53:33 -0400 Subject: [PATCH 16/16] Add more missing indexes --- 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)