From 817a2d49e58a17dc8c2716675343d7449f950842 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 12 Jan 2022 15:12:14 +0100 Subject: [PATCH 01/12] Fix staticUrlToPrefixed url rewriting --- client/src/layout/masthead.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/layout/masthead.js b/client/src/layout/masthead.js index 9fcc930e387..ec5bfffaca9 100644 --- a/client/src/layout/masthead.js +++ b/client/src/layout/masthead.js @@ -44,7 +44,7 @@ export class MastheadState { } function staticUrlToPrefixed(appRoot, url) { - return url?.startsWith("/") ? `${appRoot}${url}` : url; + return url?.startsWith("/") ? `${appRoot}${url.substring(1)}` : url; } export function mountMasthead(el, options, mastheadState) { From 855e9f9f2fb172cdcd8aae2268eee9cae2746023 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 12 Jan 2022 20:00:37 +0100 Subject: [PATCH 02/12] Fix re-run with rerun_remap_job_id hiding collection Fixes https://github.com/galaxyproject/galaxy/issues/13052 --- lib/galaxy/tools/actions/__init__.py | 3 ++- lib/galaxy_test/api/test_jobs.py | 38 +++++++++++++++++++++------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index 1b10fb20481..0411bf1924c 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -623,6 +623,8 @@ class DefaultToolAction: assert old_job.session_id == galaxy_session.id, f'({old_job.id}/{current_job.id}): Old session id ({old_job.session_id}) does not match rerun session id ({galaxy_session.id})' else: raise Exception(f'({old_job.id}/{current_job.id}): Remapping via the API is not (yet) supported') + # Start by hiding current job outputs before taking over the old job's (implicit) outputs. + current_job.hide_outputs(flush=False) # Duplicate PJAs before remap. for pjaa in old_job.post_job_actions: current_job.add_post_job_action(pjaa.post_job_action) @@ -660,7 +662,6 @@ class DefaultToolAction: job.job_id = current_job.id for jtoidca in old_job.output_dataset_collections: jtoidca.dataset_collection.replace_failed_elements(remapped_hdas) - current_job.hide_outputs(flush=False) except Exception: log.exception('Cannot remap rerun dependencies.') diff --git a/lib/galaxy_test/api/test_jobs.py b/lib/galaxy_test/api/test_jobs.py index fc6c39cebb2..2fd523196a7 100644 --- a/lib/galaxy_test/api/test_jobs.py +++ b/lib/galaxy_test/api/test_jobs.py @@ -269,18 +269,21 @@ steps: assert_ok=False) assert dataset['visible'] + def _run_map_over_error(self, history_id): + hdca1 = self.dataset_collection_populator.create_list_in_history(history_id, contents=[("sample1-1", "1 2 3")]).json() + inputs = { + 'error_bool': 'true', + 'dataset': { + 'batch': True, + 'values': [{'src': 'hdca', 'id': hdca1['id']}], + } + } + return self._run_detect_errors(history_id=history_id, inputs=inputs) + @skip_without_tool("detect_errors_aggressive") def test_no_unhide_on_error_if_mapped_over(self): with self.dataset_populator.test_history() as history_id: - hdca1 = self.dataset_collection_populator.create_list_in_history(history_id, contents=[("sample1-1", "1 2 3")]).json() - inputs = { - 'error_bool': 'true', - 'dataset': { - 'batch': True, - 'values': [{'src': 'hdca', 'id': hdca1['id']}], - } - } - run_response = self._run_detect_errors(history_id=history_id, inputs=inputs) + run_response = self._run_map_over_error(history_id) job_id = run_response['jobs'][0]["id"] self.dataset_populator.wait_for_job(job_id) job = self.dataset_populator.get_job_details(job_id).json() @@ -290,6 +293,23 @@ steps: assert_ok=False) assert not dataset['visible'] + def test_no_hide_on_rerun(self): + with self.dataset_populator.test_history() as history_id: + run_response = self._run_map_over_error(history_id) + assert run_response['implicit_collections'][0]['visible'] + job_id = run_response['jobs'][0]["id"] + rerun_params = self._get(f"jobs/{job_id}/build_for_rerun").json() + inputs = rerun_params['state_inputs'] + inputs['rerun_remap_job_id'] = job_id + self._run_detect_errors(history_id=history_id, inputs=inputs) + # Verify source hdca is still visible + hdca = self.dataset_populator.get_history_collection_details( + history_id=history_id, + content_id=run_response['implicit_collections'][0]['id'], + assert_ok=False, + ) + assert hdca['visible'] + @skip_without_tool('empty_output') def test_common_problems(self): with self.dataset_populator.test_history() as history_id: From a327e34c89cbc5e95c232cc0bf6bb3dc93e8aecb Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 12 Jan 2022 12:07:15 +0100 Subject: [PATCH 03/12] Limit lifetime of repository cache --- lib/galaxy/app.py | 6 ++--- lib/galaxy/queue_worker.py | 4 --- lib/galaxy/structured_app.py | 2 +- .../installed_repository_metadata_manager.py | 1 - lib/galaxy/tool_shed/util/repository_util.py | 25 +++++++++---------- .../tool_shed/util/tool_dependency_util.py | 1 - lib/galaxy/tool_util/toolbox/base.py | 4 ++- lib/galaxy/tools/cache.py | 12 +++------ .../galaxy/controllers/admin_toolshed.py | 3 +-- .../tools/test_tool_shed_repository_cache.py | 10 ++++---- 10 files changed, 28 insertions(+), 40 deletions(-) diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py index e8a89669ba9..db4b507609e 100644 --- a/lib/galaxy/app.py +++ b/lib/galaxy/app.py @@ -232,7 +232,6 @@ class UniverseApplication(StructuredApp, GalaxyManagerApplication): ("job manager", self._shutdown_job_manager), ("application heartbeat", self._shutdown_heartbeat), ("repository manager", self._shutdown_repo_manager), - ("database connection repository cache", self._shutdown_repo_cache), ("database connection", self._shutdown_model), ("application stack", self._shutdown_application_stack), ] @@ -384,6 +383,8 @@ class UniverseApplication(StructuredApp, GalaxyManagerApplication): self.url_for = url_for self.server_starttime = int(time.time()) # used for cachebusting + # Limit lifetime of tool shed repository cache to app startup + self.tool_shed_repository_cache = None log.info(f"Galaxy app startup finished {startup_timer}") def _shutdown_queue_worker(self): @@ -408,9 +409,6 @@ class UniverseApplication(StructuredApp, GalaxyManagerApplication): def _shutdown_repo_manager(self): self.update_repository_manager.shutdown() - def _shutdown_repo_cache(self): - self.tool_shed_repository_cache.shutdown() - def _shutdown_application_stack(self): self.application_stack.shutdown() diff --git a/lib/galaxy/queue_worker.py b/lib/galaxy/queue_worker.py index 16e7f553df7..9dc382e6e7b 100644 --- a/lib/galaxy/queue_worker.py +++ b/lib/galaxy/queue_worker.py @@ -181,8 +181,6 @@ def _get_new_toolbox(app, save_integrated_tool_panel=True): """ from galaxy import tools from galaxy.tools.special_tools import load_lib_tools - if hasattr(app, 'tool_shed_repository_cache'): - app.tool_shed_repository_cache.rebuild() tool_configs = app.config.tool_configs new_toolbox = tools.ToolBox(tool_configs, app.config.tool_path, app, save_integrated_tool_panel=save_integrated_tool_panel) @@ -199,8 +197,6 @@ def reload_data_managers(app, **kwargs): reload_timer = util.ExecutionTimer() from galaxy.tools.data_manager.manager import DataManagers log.debug("Executing data managers reload on '%s'", app.config.server_name) - if hasattr(app, 'tool_shed_repository_cache'): - app.tool_shed_repository_cache.rebuild() app._configure_tool_data_tables(from_shed_config=False) reload_tool_data_tables(app) reload_count = app.data_managers._reload_count diff --git a/lib/galaxy/structured_app.py b/lib/galaxy/structured_app.py index 715f42ceab7..642c1ea48e0 100644 --- a/lib/galaxy/structured_app.py +++ b/lib/galaxy/structured_app.py @@ -112,7 +112,7 @@ class StructuredApp(MinimalManagerApp): error_reports: Any # 'galaxy.tools.error_reports.ErrorReports' job_config: Any # 'galaxy.jobs.JobConfiguration' tool_cache: Any # 'galaxy.tools.cache.ToolCache' - tool_shed_repository_cache: Any # 'galaxy.tools.cache.ToolShedRepositoryCache' + tool_shed_repository_cache: Optional[Any] # 'galaxy.tools.cache.ToolShedRepositoryCache' watchers: Any # 'galaxy.config_watchers.ConfigWatchers' installed_repository_manager: Any # 'galaxy.tool_shed.galaxy_install.installed_repository_manager.InstalledRepositoryManager' workflow_scheduling_manager: Any # 'galaxy.workflow.scheduling_manager.WorkflowSchedulingManager' diff --git a/lib/galaxy/tool_shed/galaxy_install/metadata/installed_repository_metadata_manager.py b/lib/galaxy/tool_shed/galaxy_install/metadata/installed_repository_metadata_manager.py index 6619ef98773..72ae00b8c7f 100644 --- a/lib/galaxy/tool_shed/galaxy_install/metadata/installed_repository_metadata_manager.py +++ b/lib/galaxy/tool_shed/galaxy_install/metadata/installed_repository_metadata_manager.py @@ -94,7 +94,6 @@ class InstalledRepositoryMetadataManager(MetadataGenerator): self.update_in_shed_tool_config() self.app.install_model.context.add(self.repository) self.app.install_model.context.flush() - self.app.tool_shed_repository_cache.rebuild() log.debug(f'Metadata has been reset on repository {self.repository.name}.') else: log.debug(f'Metadata did not need to be reset on repository {self.repository.name}.') diff --git a/lib/galaxy/tool_shed/util/repository_util.py b/lib/galaxy/tool_shed/util/repository_util.py index dcad79e1ce0..ccf64076234 100644 --- a/lib/galaxy/tool_shed/util/repository_util.py +++ b/lib/galaxy/tool_shed/util/repository_util.py @@ -106,7 +106,7 @@ def create_or_update_tool_shed_repository(app, name, description, installed_chan deleted = False uninstalled = False tool_shed_repository = \ - get_installed_repository(app, tool_shed=tool_shed, name=name, owner=owner, installed_changeset_revision=installed_changeset_revision, refresh=True) + get_installed_repository(app, tool_shed=tool_shed, name=name, owner=owner, installed_changeset_revision=installed_changeset_revision) if tool_shed_repository: log.debug("Updating an existing row for repository '%s' in the tool_shed_repository table, status set to '%s'.", name, status) tool_shed_repository.description = description @@ -208,15 +208,17 @@ def get_installed_repository(app, tool_shed=None, name=None, owner=None, changes """ # We store the port, if one exists, in the database. tool_shed = common_util.remove_protocol_from_tool_shed_url(tool_shed) - if from_cache and hasattr(app, 'tool_shed_repository_cache'): - if refresh: - app.tool_shed_repository_cache.rebuild() - return app.tool_shed_repository_cache.get_installed_repository(tool_shed=tool_shed, - name=name, - owner=owner, - installed_changeset_revision=installed_changeset_revision, - changeset_revision=changeset_revision, - repository_id=repository_id) + if from_cache: + tsr_cache = getattr(app, 'tool_shed_repository_cache', None) + if tsr_cache: + return app.tool_shed_repository_cache.get_installed_repository( + tool_shed=tool_shed, + name=name, + owner=owner, + installed_changeset_revision=installed_changeset_revision, + changeset_revision=changeset_revision, + repository_id=repository_id + ) query = app.install_model.context.query(app.install_model.ToolShedRepository) if repository_id: clause_list = [app.install_model.ToolShedRepository.table.c.id == repository_id] @@ -239,8 +241,6 @@ def get_installed_tool_shed_repository(app, id): else: id = [id] return_list = False - if hasattr(app, 'tool_shed_repository_cache'): - app.tool_shed_repository_cache.rebuild() repository_ids = [app.security.decode_id(i) for i in id] rval = [get_installed_repository(app=app, repository_id=repo_id, from_cache=False) for repo_id in repository_ids] if return_list: @@ -377,7 +377,6 @@ def get_repository_for_dependency_relationship(app, tool_shed, name, owner, chan message += "required parameters is None: tool_shed: %s, name: %s, owner: %s, changeset_revision: %s " % \ (str(tool_shed), str(name), str(owner), str(changeset_revision)) raise Exception(message) - app.tool_shed_repository_cache.rebuild() repository = get_installed_repository(app=app, tool_shed=tool_shed, name=name, diff --git a/lib/galaxy/tool_shed/util/tool_dependency_util.py b/lib/galaxy/tool_shed/util/tool_dependency_util.py index 7105bb92f16..318cec119c7 100644 --- a/lib/galaxy/tool_shed/util/tool_dependency_util.py +++ b/lib/galaxy/tool_shed/util/tool_dependency_util.py @@ -388,5 +388,4 @@ def set_tool_dependency_attributes(app, tool_dependency, status, error_message=N tool_dependency.status = status sa_session.add(tool_dependency) sa_session.flush() - app.tool_shed_repository_cache.rebuild() return tool_dependency diff --git a/lib/galaxy/tool_util/toolbox/base.py b/lib/galaxy/tool_util/toolbox/base.py index ed58ec431e3..7fc3f810225 100644 --- a/lib/galaxy/tool_util/toolbox/base.py +++ b/lib/galaxy/tool_util/toolbox/base.py @@ -807,7 +807,9 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): repository_path, tool_path ) - self.app.tool_shed_repository_cache.add_local_repository(repository) + tsr_cache = self.app.tool_shed_repository_cache + if tsr_cache: + tsr_cache.add_local_repository(repository) return repository def _get_tool_shed_repository(self, tool_shed, name, owner, installed_changeset_revision): diff --git a/lib/galaxy/tools/cache.py b/lib/galaxy/tools/cache.py index ca23c0768c8..22f82fe488c 100644 --- a/lib/galaxy/tools/cache.py +++ b/lib/galaxy/tools/cache.py @@ -13,7 +13,6 @@ from sqlalchemy.orm import ( defer, joinedload, ) -from sqlalchemy.orm.scoping import scoped_session from sqlalchemy.orm.session import sessionmaker from sqlitedict import SqliteDict @@ -288,20 +287,20 @@ class ToolShedRepositoryCache: repos_by_tuple: Dict[Tuple[str, str, str], List[ToolConfRepository]] def __init__(self, session: sessionmaker): - engine = session().bind - self.session = scoped_session(sessionmaker(engine)) + self.session = session() # Contains ToolConfRepository objects created from shed_tool_conf.xml entries self.local_repositories = [] # Repositories loaded from database self.repositories = [] self.repos_by_tuple = defaultdict(list) - self.rebuild() + self._rebuild() + self.session.close() def add_local_repository(self, repository): self.local_repositories.append(repository) self.repos_by_tuple[(repository.tool_shed, repository.owner, repository.name)].append(repository) - def rebuild(self): + def _rebuild(self): self.repositories = self.session.query(ToolShedRepository).options( defer(ToolShedRepository.metadata), joinedload('tool_dependencies') ).all() @@ -325,6 +324,3 @@ class ToolShedRepositoryCache: continue return repo return None - - def shutdown(self) -> None: - self.session.close() diff --git a/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py b/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py index 7e92798353c..8ef04b5b1f0 100644 --- a/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py +++ b/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py @@ -1241,8 +1241,7 @@ class AdminToolshed(AdminGalaxy): tool_shed=tool_shed_url, name=name, owner=owner, - changeset_revision=changeset_revision, - refresh=True) + changeset_revision=changeset_revision) if changeset_revision and latest_changeset_revision and latest_ctx_rev: if changeset_revision == latest_changeset_revision: message = f"The installed repository named '{name}' is current, there are no updates available. " diff --git a/test/unit/tools/test_tool_shed_repository_cache.py b/test/unit/tools/test_tool_shed_repository_cache.py index 26966703b07..9b4501f4f4d 100644 --- a/test/unit/tools/test_tool_shed_repository_cache.py +++ b/test/unit/tools/test_tool_shed_repository_cache.py @@ -9,21 +9,21 @@ def test_empty_repo_cache(tool_shed_repository_cache): def test_add_repository_to_repository_cache(tool_shed_repository_cache, repos): - tool_shed_repository_cache.rebuild() + tool_shed_repository_cache._rebuild() assert len(tool_shed_repository_cache.repositories) == 10 assert len(tool_shed_repository_cache.local_repositories) == 0 def test_add_repository_and_tool_conf_repository_to_repository_cache(tool_shed_repository_cache, repos, tool_conf_repos): - tool_shed_repository_cache.rebuild() + tool_shed_repository_cache._rebuild() assert len(tool_shed_repository_cache.repositories) == 10 assert len(tool_shed_repository_cache.local_repositories) == 10 - tool_shed_repository_cache.rebuild() + tool_shed_repository_cache._rebuild() assert len(tool_shed_repository_cache.repositories) == 10 assert len(tool_shed_repository_cache.local_repositories) == 10 create_repo(tool_shed_repository_cache.session, '21', '20') tool_shed_repository_cache.session.flush() - tool_shed_repository_cache.rebuild() + tool_shed_repository_cache._rebuild() assert len(tool_shed_repository_cache.repositories) == 11 assert len(tool_shed_repository_cache.local_repositories) == 10 @@ -41,7 +41,7 @@ def test_add_repository_and_tool_conf_repository_to_repository_cache(tool_shed_r ('github.com', 'example', 'galaxyproject', '19', '18', None, True), ]) def test_get_installed_repository(tool_shed_repository_cache, repos, tool_conf_repos, tool_shed, name, owner, changeset_revision, installed_changeset_revision, repository_id, repo_exists): - tool_shed_repository_cache.rebuild() + tool_shed_repository_cache._rebuild() repo = tool_shed_repository_cache.get_installed_repository( tool_shed=tool_shed, name=name, From afd94218f03e4fd507ba42a77bb33f649b382874 Mon Sep 17 00:00:00 2001 From: Marius van den Beek Date: Wed, 12 Jan 2022 19:48:16 +0100 Subject: [PATCH 04/12] Don't need to use a joinedload anymore The cache is only used on startup, not during dependency resolution. Also rename _rebuild, since we only use it once now. --- lib/galaxy/tools/cache.py | 11 ++++------- test/unit/tools/test_tool_shed_repository_cache.py | 10 +++++----- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/tools/cache.py b/lib/galaxy/tools/cache.py index 22f82fe488c..12e571e018d 100644 --- a/lib/galaxy/tools/cache.py +++ b/lib/galaxy/tools/cache.py @@ -9,10 +9,7 @@ from collections import defaultdict from threading import Lock from typing import Dict, List, Tuple -from sqlalchemy.orm import ( - defer, - joinedload, -) +from sqlalchemy.orm import defer from sqlalchemy.orm.session import sessionmaker from sqlitedict import SqliteDict @@ -293,16 +290,16 @@ class ToolShedRepositoryCache: # Repositories loaded from database self.repositories = [] self.repos_by_tuple = defaultdict(list) - self._rebuild() + self._build() self.session.close() def add_local_repository(self, repository): self.local_repositories.append(repository) self.repos_by_tuple[(repository.tool_shed, repository.owner, repository.name)].append(repository) - def _rebuild(self): + def _build(self): self.repositories = self.session.query(ToolShedRepository).options( - defer(ToolShedRepository.metadata), joinedload('tool_dependencies') + defer(ToolShedRepository.metadata) ).all() repos_by_tuple = defaultdict(list) for repository in self.repositories + self.local_repositories: diff --git a/test/unit/tools/test_tool_shed_repository_cache.py b/test/unit/tools/test_tool_shed_repository_cache.py index 9b4501f4f4d..436147303ca 100644 --- a/test/unit/tools/test_tool_shed_repository_cache.py +++ b/test/unit/tools/test_tool_shed_repository_cache.py @@ -9,21 +9,21 @@ def test_empty_repo_cache(tool_shed_repository_cache): def test_add_repository_to_repository_cache(tool_shed_repository_cache, repos): - tool_shed_repository_cache._rebuild() + tool_shed_repository_cache._build() assert len(tool_shed_repository_cache.repositories) == 10 assert len(tool_shed_repository_cache.local_repositories) == 0 def test_add_repository_and_tool_conf_repository_to_repository_cache(tool_shed_repository_cache, repos, tool_conf_repos): - tool_shed_repository_cache._rebuild() + tool_shed_repository_cache._build() assert len(tool_shed_repository_cache.repositories) == 10 assert len(tool_shed_repository_cache.local_repositories) == 10 - tool_shed_repository_cache._rebuild() + tool_shed_repository_cache._build() assert len(tool_shed_repository_cache.repositories) == 10 assert len(tool_shed_repository_cache.local_repositories) == 10 create_repo(tool_shed_repository_cache.session, '21', '20') tool_shed_repository_cache.session.flush() - tool_shed_repository_cache._rebuild() + tool_shed_repository_cache._build() assert len(tool_shed_repository_cache.repositories) == 11 assert len(tool_shed_repository_cache.local_repositories) == 10 @@ -41,7 +41,7 @@ def test_add_repository_and_tool_conf_repository_to_repository_cache(tool_shed_r ('github.com', 'example', 'galaxyproject', '19', '18', None, True), ]) def test_get_installed_repository(tool_shed_repository_cache, repos, tool_conf_repos, tool_shed, name, owner, changeset_revision, installed_changeset_revision, repository_id, repo_exists): - tool_shed_repository_cache._rebuild() + tool_shed_repository_cache._build() repo = tool_shed_repository_cache.get_installed_repository( tool_shed=tool_shed, name=name, From bf712670f2d62673da16bd88916c48d43abe7cb3 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 13 Jan 2022 13:35:36 +0100 Subject: [PATCH 05/12] Drop refresh argument of get_installed_repository --- lib/galaxy/tool_shed/util/repository_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tool_shed/util/repository_util.py b/lib/galaxy/tool_shed/util/repository_util.py index ccf64076234..e4ac37fc334 100644 --- a/lib/galaxy/tool_shed/util/repository_util.py +++ b/lib/galaxy/tool_shed/util/repository_util.py @@ -201,7 +201,7 @@ def get_ids_of_tool_shed_repositories_being_installed(app, as_string=False): return installing_repository_ids -def get_installed_repository(app, tool_shed=None, name=None, owner=None, changeset_revision=None, installed_changeset_revision=None, repository_id=None, refresh=False, from_cache=False): +def get_installed_repository(app, tool_shed=None, name=None, owner=None, changeset_revision=None, installed_changeset_revision=None, repository_id=None, from_cache=False): """ Return a tool shed repository database record defined by the combination of a toolshed, repository name, repository owner and either current or originally installed changeset_revision. From fc8fb8553097d766fb8a7fdf33496c73368ab136 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 14 Jan 2022 11:49:04 +0100 Subject: [PATCH 06/12] Set collection update_time when replacing failed elements That allows the new history to update the collection. --- lib/galaxy/model/__init__.py | 28 ++++++++++++++++------------ lib/galaxy/tools/actions/__init__.py | 1 + lib/galaxy_test/api/test_jobs.py | 15 +++++++++++++-- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index ad671eaf389..d179ecaadc6 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -295,15 +295,20 @@ class HasName: class UsesCreateAndUpdateTime: + update_time: DateTime + @property def seconds_since_updated(self): - update_time = self.update_time or galaxy.model.orm.now.now() # In case not yet flushed - return (galaxy.model.orm.now.now() - update_time).total_seconds() + update_time = self.update_time or now() # In case not yet flushed + return (now() - update_time).total_seconds() @property def seconds_since_created(self): - create_time = self.create_time or galaxy.model.orm.now.now() # In case not yet flushed - return (galaxy.model.orm.now.now() - create_time).total_seconds() + create_time = self.create_time or now() # In case not yet flushed + return (now() - create_time).total_seconds() + + def update(self): + self.update_time = now() class WorkerProcess(Base, UsesCreateAndUpdateTime, _HasTable): @@ -776,7 +781,7 @@ class PasswordResetToken(Base, _HasTable): else: self.token = unique_id() self.user = user - self.expiration_time = galaxy.model.orm.now.now() + timedelta(hours=24) + self.expiration_time = now() + timedelta(hours=24) class DynamicTool(Base, Dictifiable, RepresentById): @@ -1452,7 +1457,7 @@ class Job(Base, JobLike, UsesCreateAndUpdateTime, Dictifiable, RepresentById): WHERE job_id = :job_id; ''' sa_session = object_session(self) - update_time = galaxy.model.orm.now.now() + update_time = now() self.update_hdca_update_time_for_job(update_time=update_time, sa_session=sa_session, supports_skip_locked=supports_skip_locked) params = { 'job_id': self.id, @@ -1524,7 +1529,7 @@ class Job(Base, JobLike, UsesCreateAndUpdateTime, Dictifiable, RepresentById): ); '''] sa_session = object_session(self) - update_time = galaxy.model.orm.now.now() + update_time = now() self.update_hdca_update_time_for_job(update_time=update_time, sa_session=sa_session, supports_skip_locked=supports_skip_locked) params = { 'job_id': self.id, @@ -3466,7 +3471,7 @@ def datatype_for_extension(extension, datatypes_registry=None): return ret -class DatasetInstance: +class DatasetInstance(UsesCreateAndUpdateTime): """A base class for all 'dataset instances', HDAs, LDAs, etc""" states = Dataset.states conversion_messages = Dataset.conversion_messages @@ -3519,9 +3524,6 @@ class DatasetInstance: def peek(self, peek): self._peek = unicodify(peek, strip_null=True) - def update(self): - self.update_time = galaxy.model.orm.now.now() - @property def ext(self): return self.extension @@ -5430,7 +5432,7 @@ class DatasetCollection(Base, Dictifiable, UsesAnnotations, RepresentById): return rval -class DatasetCollectionInstance(HasName): +class DatasetCollectionInstance(HasName, UsesCreateAndUpdateTime): @property def state(self): @@ -5692,6 +5694,8 @@ class HistoryDatasetCollectionAssociation( deleted=self.deleted, job_source_id=self.job_source_id, job_source_type=self.job_source_type, + create_time=self.create_time.isoformat(), + update_time=self.update_time.isoformat(), **self._base_to_dict(view=view) ) diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index 0411bf1924c..d622df5324d 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -660,6 +660,7 @@ class DefaultToolAction: for job in hdca.implicit_collection_jobs.jobs: if job.job_id == old_job.id: job.job_id = current_job.id + hdca.update() for jtoidca in old_job.output_dataset_collections: jtoidca.dataset_collection.replace_failed_elements(remapped_hdas) except Exception: diff --git a/lib/galaxy_test/api/test_jobs.py b/lib/galaxy_test/api/test_jobs.py index 2fd523196a7..09f5d732193 100644 --- a/lib/galaxy_test/api/test_jobs.py +++ b/lib/galaxy_test/api/test_jobs.py @@ -5,6 +5,7 @@ import time from operator import itemgetter import requests +from dateutil.parser import isoparse from galaxy_test.api.test_tools import TestsTools from galaxy_test.base.api_asserts import assert_status_code_is_ok @@ -296,12 +297,21 @@ steps: def test_no_hide_on_rerun(self): with self.dataset_populator.test_history() as history_id: run_response = self._run_map_over_error(history_id) - assert run_response['implicit_collections'][0]['visible'] job_id = run_response['jobs'][0]["id"] + self.dataset_populator.wait_for_job(job_id) + failed_hdca = self.dataset_populator.get_history_collection_details( + history_id=history_id, + content_id=run_response['implicit_collections'][0]['id'], + assert_ok=False, + ) + first_update_time = failed_hdca['update_time'] + assert failed_hdca['visible'] rerun_params = self._get(f"jobs/{job_id}/build_for_rerun").json() inputs = rerun_params['state_inputs'] inputs['rerun_remap_job_id'] = job_id - self._run_detect_errors(history_id=history_id, inputs=inputs) + rerun_response = self._run_detect_errors(history_id=history_id, inputs=inputs) + rerun_job_id = rerun_response['jobs'][0]["id"] + self.dataset_populator.wait_for_job(rerun_job_id) # Verify source hdca is still visible hdca = self.dataset_populator.get_history_collection_details( history_id=history_id, @@ -309,6 +319,7 @@ steps: assert_ok=False, ) assert hdca['visible'] + assert isoparse(hdca['update_time']) > (isoparse(first_update_time)) @skip_without_tool('empty_output') def test_common_problems(self): From ea24c7c93e3485c5c578a594bd46730af4c7aa25 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 17 Jan 2022 10:14:28 +0100 Subject: [PATCH 07/12] Show yaml files in preview The `application/yaml` mediatype might be the logical mediatype for yaml (there's no standard yet), but we want to see a textual preview (in the absence of a viewer). Fixes https://github.com/galaxyproject/galaxy/issues/13168 --- lib/galaxy/datatypes/text.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/galaxy/datatypes/text.py b/lib/galaxy/datatypes/text.py index 46e7f07dd09..091965dafd8 100644 --- a/lib/galaxy/datatypes/text.py +++ b/lib/galaxy/datatypes/text.py @@ -875,6 +875,13 @@ class Yaml(Text): """Returns the mime type of the datatype""" return 'application/yaml' + def _yield_user_file_content(self, trans, from_dataset, filename): + # Override non-standard application/yaml mediatype with + # non-standard text/x-yaml, so preview is shown in preview iframe, + # instead of downloading the file. + trans.response.set_content_type('text/x-yaml') + return super()._yield_user_file_content(trans, from_dataset, filename) + def _looks_like_yaml(self, file_prefix): # Pattern used by SequenceSplitLocations if file_prefix.file_size < 50000 and not file_prefix.truncated: From 9e546f7d97781b7990e8d17db16d421f5abcbdd5 Mon Sep 17 00:00:00 2001 From: Ruben Vorderman Date: Mon, 17 Jan 2022 16:02:01 +0100 Subject: [PATCH 08/12] Catch exceptions when job.user is None prevents the following error: galaxy.web.framework.decorators ERROR 2022-01-17 14:52:09,619 [p:18,w:1,m:0] [uWSGIWorker1Core1] Uncaught exception in exposed API method: Traceback (most recent call last): File "lib/galaxy/web/framework/decorators.py", line 282, in decorator rval = func(self, trans, *args, **kwargs) File "lib/galaxy/webapps/galaxy/api/jobs.py", line 113, in index j['user_email'] = job.user.email AttributeError: 'NoneType' object has no attribute 'email' --- lib/galaxy/webapps/galaxy/api/jobs.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py index a63f2efcdae..789b23846a7 100644 --- a/lib/galaxy/webapps/galaxy/api/jobs.py +++ b/lib/galaxy/webapps/galaxy/api/jobs.py @@ -210,7 +210,10 @@ class JobController(BaseGalaxyAPIController, UsesVisualizationMixin): if view == 'admin_job_list': j['decoded_job_id'] = job.id if user_details: - j['user_email'] = job.user.email + try: + j['user_email'] = job.user.email + except AttributeError: # when job.user is None + j['user_email'] = None out.append(j) return out From faddbeeca917748539060022177ad3f54861d3d8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 18 Jan 2022 14:20:50 +0100 Subject: [PATCH 09/12] Use job.get_user_email systematically --- lib/galaxy/jobs/__init__.py | 9 +++------ lib/galaxy/jobs/actions/post.py | 2 +- lib/galaxy/managers/jobs.py | 6 +----- lib/galaxy/model/__init__.py | 9 +++++++++ lib/galaxy/webapps/galaxy/api/jobs.py | 5 +---- lib/galaxy/webapps/reports/controllers/jobs.py | 7 ++----- templates/webapps/reports/job_info.mako | 4 ++-- 7 files changed, 19 insertions(+), 23 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index ca641f24dd8..82614ed7b42 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -2221,12 +2221,9 @@ class JobWrapper(HasResourceParameters): @property def user(self): job = self.get_job() - if job.user is not None: - return job.user.email - elif job.galaxy_session is not None and job.galaxy_session.user is not None: - return job.galaxy_session.user.email - elif job.history is not None and job.history.user is not None: - return job.history.user.email + user_email = job.get_user_email() + if user_email: + return user_email elif job.galaxy_session is not None: return f"anonymous@{job.galaxy_session.remote_addr.split()[-1]}" else: diff --git a/lib/galaxy/jobs/actions/post.py b/lib/galaxy/jobs/actions/post.py index 1f9deb2620a..9ad759aaba7 100644 --- a/lib/galaxy/jobs/actions/post.py +++ b/lib/galaxy/jobs/actions/post.py @@ -54,7 +54,7 @@ class EmailAction(DefaultJobAction): else: host = socket.getfqdn() frm = f'galaxy-no-reply@{host}' - to = job.user.email + to = job.get_user_email() subject = f"Galaxy job completion notification from history '{job.history.name}'" outdata = ',\n'.join(ds.dataset.display_name() for ds in job.output_datasets) body = f"Your Galaxy job generating dataset(s):\n\n{outdata}\n\nis complete as of {datetime.datetime.now().strftime('%I:%M')}. Click the link below to access your data: \n{link}" diff --git a/lib/galaxy/managers/jobs.py b/lib/galaxy/managers/jobs.py index 85be165788b..c8528e4a79c 100644 --- a/lib/galaxy/managers/jobs.py +++ b/lib/galaxy/managers/jobs.py @@ -411,11 +411,7 @@ def view_show_job(trans, job, full: bool) -> typing.Dict: )) if is_admin: - if job.user: - job_dict['user_email'] = job.user.email - else: - job_dict['user_email'] = None - + job_dict['user_email'] = job.get_user_email() job_dict['job_metrics'] = summarize_job_metrics(trans, job) return job_dict diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index d179ecaadc6..328bb841a55 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -1125,6 +1125,15 @@ class Job(Base, JobLike, UsesCreateAndUpdateTime, Dictifiable, RepresentById): def set_tool_id(self, tool_id): self.tool_id = tool_id + def get_user_email(self): + if self.user is not None: + return self.user.email + elif self.galaxy_session is not None and self.galaxy_session.user is not None: + return self.galaxy_session.user.email + elif self.history is not None and self.history.user is not None: + return self.history.user.email + return None + def set_tool_version(self, tool_version): self.tool_version = tool_version diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py index 789b23846a7..b70837c8012 100644 --- a/lib/galaxy/webapps/galaxy/api/jobs.py +++ b/lib/galaxy/webapps/galaxy/api/jobs.py @@ -210,10 +210,7 @@ class JobController(BaseGalaxyAPIController, UsesVisualizationMixin): if view == 'admin_job_list': j['decoded_job_id'] = job.id if user_details: - try: - j['user_email'] = job.user.email - except AttributeError: # when job.user is None - j['user_email'] = None + j['user_email'] = job.get_user_email() out.append(j) return out diff --git a/lib/galaxy/webapps/reports/controllers/jobs.py b/lib/galaxy/webapps/reports/controllers/jobs.py index d5cc9d3e117..e9dbe74dea5 100644 --- a/lib/galaxy/webapps/reports/controllers/jobs.py +++ b/lib/galaxy/webapps/reports/controllers/jobs.py @@ -143,7 +143,7 @@ class SpecifiedDateListGrid(grids.Grid): def get_value(self, trans, grid, job): if job.user: - return escape(job.user.email) + return escape(job.get_user_email()) return 'anonymous' class EmailColumn(grids.GridColumn): @@ -279,10 +279,7 @@ class Jobs(BaseUIController, ReportQueryBuilder): # that submitted the job. job_id = kwd.get('id', None) job = get_job(trans, job_id) - if job.user: - kwd['email'] = job.user.email - else: - kwd['email'] = None # For anonymous users + kwd['email'] = job.get_user_email() return trans.response.send_redirect(web.url_for(controller='jobs', action='user_per_month', **kwd)) diff --git a/templates/webapps/reports/job_info.mako b/templates/webapps/reports/job_info.mako index 9b844e9d90e..41f016175b4 100644 --- a/templates/webapps/reports/job_info.mako +++ b/templates/webapps/reports/job_info.mako @@ -37,8 +37,8 @@ ${job.tool_id} - %if job.user and job.user.email: - ${job.user.email} + %if job.get_user_email(): + ${job.get_user_email()} %else: anonymous %endif From 28423d27efcc2981c23a4356a3ddbf3f5052fb06 Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Thu, 13 Jan 2022 15:19:10 -0500 Subject: [PATCH 10/12] Clear the cache before fetching a file from irods object store, to verify file is fetched from the object store, not from the cache --- .../objectstore/test_objectstore_datatype_upload.py | 2 +- test/integration/test_datatype_upload.py | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/test/integration/objectstore/test_objectstore_datatype_upload.py b/test/integration/objectstore/test_objectstore_datatype_upload.py index 52178898be8..de0a24a2091 100644 --- a/test/integration/objectstore/test_objectstore_datatype_upload.py +++ b/test/integration/objectstore/test_objectstore_datatype_upload.py @@ -183,7 +183,7 @@ def test_upload_datatype_dos_disk_and_disk(distributed_instance, test_data, temp @pytest.mark.parametrize('test_data', TEST_CASES.values(), ids=list(TEST_CASES.keys())) def test_upload_datatype_irods(irods_instance, test_data, temp_file): - upload_datatype_helper(irods_instance, test_data, temp_file) + upload_datatype_helper(irods_instance, test_data, temp_file, True) @pytest.mark.parametrize('test_data', TEST_CASES.values(), ids=list(TEST_CASES.keys())) diff --git a/test/integration/test_datatype_upload.py b/test/integration/test_datatype_upload.py index e8c46ae17db..2e2a0485ed6 100644 --- a/test/integration/test_datatype_upload.py +++ b/test/integration/test_datatype_upload.py @@ -1,5 +1,6 @@ import collections import os +import shutil import pytest @@ -59,7 +60,7 @@ def test_upload_datatype_auto(instance, test_data, temp_file): upload_datatype_helper(instance, test_data, temp_file) -def upload_datatype_helper(instance, test_data, temp_file): +def upload_datatype_helper(instance, test_data, temp_file, delete_cache_dir=False): is_compressed = False for is_method in (is_bz2, is_gzip, is_zip): is_compressed = is_method(test_data.path) @@ -93,6 +94,14 @@ def upload_datatype_helper(instance, test_data, temp_file): datatype = registry.datatypes_by_extension[file_ext] datatype_compressed = getattr(datatype, "compressed", False) if not is_compressed or datatype_compressed: + if delete_cache_dir: + # Delete cache directory and then re-create it. This way we confirm + # that dataset is fetched from the object store, not from the cache + temp_dir = instance.get_object_store_kwargs()['temp_directory'] + cache_dir = temp_dir + '/object_store_cache' + shutil.rmtree(cache_dir) + os.mkdir(cache_dir) + # download file and verify it hasn't been manipulated temp_file.write(instance.dataset_populator.get_history_dataset_content(history_id=instance.history_id, dataset=dataset, From 2b70e34a61e024ea6bdb0b08ec7bd51998559b90 Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Thu, 6 Jan 2022 09:41:28 -0500 Subject: [PATCH 11/12] Revised code based on code review --- lib/galaxy/objectstore/irods.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/objectstore/irods.py b/lib/galaxy/objectstore/irods.py index eb6841f6c51..9b9bfa4bf5c 100644 --- a/lib/galaxy/objectstore/irods.py +++ b/lib/galaxy/objectstore/irods.py @@ -429,7 +429,7 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): log.debug("Pushing cache file '%s' of size %s bytes to collection '%s'", source_file, os.path.getsize(source_file), rel_path) # Add the source file to the irods collection - self.session.data_objects.put(source_file, f"{collection_path}/", **options) + self.session.data_objects.put(source_file, data_object_path, **options) end_time = datetime.now() log.debug("Pushed cache file '%s' to collection '%s' (%s bytes transfered in %s sec)", From 98b7b120b564265c029d5fa40b4cb7105d1e214a Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 13 Jan 2022 13:39:05 +0100 Subject: [PATCH 12/12] Fix metadata source access in tool shed Fixes: ``` Unexpected HTTP status code: 500: {"err_msg": "Metadata may have been defined for some items in revision '8f1b150a2487'. Correct the following problems if necessary and reset metadata.
abyss-pe.xml<\/b> - 'NoneType' object has no attribute 'get_biotools_metadata'
"} ``` in https://github.com/galaxyproject/tools-iuc/runs/4792653893?check_suite_focus=true --- lib/galaxy/tools/__init__.py | 5 +++-- test/unit/unittest_utils/galaxy_mock.py | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index f2af91aa26a..d3277abfbda 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1022,8 +1022,9 @@ class Tool(Dictifiable): has_missing_data = len(edam_operations) == 0 or len(edam_topics) == 0 if has_missing_data: biotools_reference = self.biotools_reference - if biotools_reference: - biotools_entry = self.app.biotools_metadata_source.get_biotools_metadata(biotools_reference) + metadata_source = self.app.biotools_metadata_source + if biotools_reference and metadata_source: + biotools_entry = metadata_source.get_biotools_metadata(biotools_reference) if biotools_entry: edam_info = biotools_entry.edam_info if len(edam_operations) == 0: diff --git a/test/unit/unittest_utils/galaxy_mock.py b/test/unit/unittest_utils/galaxy_mock.py index 2e50ed279f5..381bbb57c7a 100644 --- a/test/unit/unittest_utils/galaxy_mock.py +++ b/test/unit/unittest_utils/galaxy_mock.py @@ -103,6 +103,7 @@ class MockApp(di.Container): self.user_manager = UserManager(self) self.execution_timer_factory = Bunch(get_timer=StructuredExecutionTimer) self.is_job_handler = False + self.biotools_metadata_source = None rebind_container_to_task(self) def url_for(*args, **kwds):