mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Keep separate session for ToolShedRepositoryCache
Should fix
```
Traceback (most recent call last):
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/jobs/runners/__init__.py", line 244, in prepare_job
stderr_file=stderr_file,
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/jobs/runners/__init__.py", line 285, in build_command_line
stderr_file=stderr_file,
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/jobs/command_factory.py", line 79, in build_command
__handle_dependency_resolution(commands_builder, job_wrapper, remote_command_params)
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/jobs/command_factory.py", line 194, in __handle_dependency_resolution
if local_dependency_resolution and job_wrapper.dependency_shell_commands:
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/jobs/__init__.py", line 1012, in dependency_shell_commands
job_directory=self.working_directory
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/tools/__init__.py", line 1891, in build_dependency_shell_commands
tool_instance=self
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/tool_util/deps/__init__.py", line 184, in dependency_shell_commands
requirements_to_dependencies = self.requirements_to_dependencies(requirements, **kwds)
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/tool_util/deps/__init__.py", line 194, in requirements_to_dependencies
requirement_to_dependency = self._requirements_to_dependencies_dict(requirements, **kwds)
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/tool_util/deps/__init__.py", line 283, in _requirements_to_dependencies_dict
dependency = resolver.resolve(requirement, **kwds)
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/tool_util/deps/resolvers/galaxy_packages.py", line 79, in resolve
return self._find_dep_versioned(name, version, type=type, **kwds)
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/tool_util/deps/resolvers/tool_shed_packages.py", line 25, in _find_dep_versioned
path = self._get_package_installed_dependency_path(installed_tool_dependency, name, version)
File "/cvmfs/test.galaxyproject.org/galaxy/lib/galaxy/tool_util/deps/resolvers/tool_shed_packages.py", line 43, in _get_package_installed_dependency_path
tool_shed_repository = installed_tool_dependency.tool_shed_repository
File "/cvmfs/test.galaxyproject.org/venv/lib/python3.6/site-packages/sqlalchemy/orm/attributes.py", line 480, in __get__
return self.impl.get(state, dict_)
File "/cvmfs/test.galaxyproject.org/venv/lib/python3.6/site-packages/sqlalchemy/orm/attributes.py", line 931, in get
value = self.callable_(state, passive)
File "/cvmfs/test.galaxyproject.org/venv/lib/python3.6/site-packages/sqlalchemy/orm/strategies.py", line 836, in _load_for_state
% (orm_util.state_str(state), self.key)
sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <ToolDependency at 0x7fd3e5e82160> is not bound to a Session; lazy load operation of attribute 'tool_shed_repository' cannot proceed (Background on this error at: http://sqlalche.me/e/14/bhk3)
```
Which I think broke in
https://github.com/galaxyproject/galaxy/pull/11737#discussion_r636354063.
Instead of hoping that we preload all attributes it seems more robust to
use a separate session for the cache that is not tied to a thread and
that will not be closed by flushes in other parts of the application.
This commit is contained in:
@@ -216,6 +216,7 @@ 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),
|
||||
]
|
||||
@@ -391,6 +392,9 @@ 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()
|
||||
|
||||
|
||||
@@ -86,7 +86,12 @@ def build_engine(url, engine_options, database_query_profiling_proxy=False, trac
|
||||
except AttributeError:
|
||||
pass
|
||||
|
||||
# Set check_same_thread to False for sqlite, handled by request-specific session
|
||||
# See https://fastapi.tiangolo.com/tutorial/sql-databases/#note
|
||||
connect_args = {}
|
||||
if 'sqlite://' in url:
|
||||
connect_args['check_same_thread'] = False
|
||||
# Create the database engine
|
||||
engine = create_engine(url, **engine_options)
|
||||
engine = create_engine(url, connect_args=connect_args, **engine_options)
|
||||
register_after_fork(engine, lambda e: e.dispose())
|
||||
return engine
|
||||
|
||||
+13
-14
@@ -13,10 +13,10 @@ from sqlalchemy.orm import (
|
||||
defer,
|
||||
joinedload,
|
||||
)
|
||||
from sqlalchemy.orm.session import sessionmaker
|
||||
from sqlitedict import SqliteDict
|
||||
|
||||
from galaxy.model.tool_shed_install import ToolShedRepository
|
||||
from galaxy.structured_app import MinimalManagerApp
|
||||
from galaxy.tool_util.toolbox.base import ToolConfRepository
|
||||
from galaxy.util import unicodify
|
||||
from galaxy.util.hash_util import md5_hash_file
|
||||
@@ -286,8 +286,8 @@ class ToolShedRepositoryCache:
|
||||
repositories: List[ToolShedRepository]
|
||||
repos_by_tuple: Dict[Tuple[str, str, str], List[ToolConfRepository]]
|
||||
|
||||
def __init__(self, app: MinimalManagerApp):
|
||||
self.app = app
|
||||
def __init__(self, session: sessionmaker):
|
||||
self.session = session()
|
||||
# Contains ToolConfRepository objects created from shed_tool_conf.xml entries
|
||||
self.local_repositories = []
|
||||
# Repositories loaded from database
|
||||
@@ -300,17 +300,13 @@ class ToolShedRepositoryCache:
|
||||
self.repos_by_tuple[(repository.tool_shed, repository.owner, repository.name)].append(repository)
|
||||
|
||||
def rebuild(self):
|
||||
try:
|
||||
session = self.app.install_model._SessionLocal()
|
||||
self.repositories = session.query(ToolShedRepository).options(
|
||||
defer(ToolShedRepository.metadata), joinedload('tool_dependencies')
|
||||
).all()
|
||||
repos_by_tuple = defaultdict(list)
|
||||
for repository in self.repositories + self.local_repositories:
|
||||
repos_by_tuple[(repository.tool_shed, repository.owner, repository.name)].append(repository)
|
||||
self.repos_by_tuple = repos_by_tuple
|
||||
finally:
|
||||
session.close()
|
||||
self.repositories = self.session.query(ToolShedRepository).options(
|
||||
defer(ToolShedRepository.metadata), joinedload('tool_dependencies')
|
||||
).all()
|
||||
repos_by_tuple = defaultdict(list)
|
||||
for repository in self.repositories + self.local_repositories:
|
||||
repos_by_tuple[(repository.tool_shed, repository.owner, repository.name)].append(repository)
|
||||
self.repos_by_tuple = repos_by_tuple
|
||||
|
||||
def get_installed_repository(self, tool_shed=None, name=None, owner=None, installed_changeset_revision=None, changeset_revision=None, repository_id=None):
|
||||
if repository_id:
|
||||
@@ -327,3 +323,6 @@ class ToolShedRepositoryCache:
|
||||
continue
|
||||
return repo
|
||||
return None
|
||||
|
||||
def shutdown(self) -> None:
|
||||
self.session.close()
|
||||
|
||||
@@ -18,13 +18,13 @@ def mock_app():
|
||||
|
||||
@pytest.fixture
|
||||
def tool_shed_repository_cache(mock_app):
|
||||
tool_shed_repository_cache = ToolShedRepositoryCache(app=mock_app)
|
||||
tool_shed_repository_cache = ToolShedRepositoryCache(session=mock_app.install_model.context)
|
||||
return tool_shed_repository_cache
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def repos(mock_app):
|
||||
repositories = [create_repo(mock_app, changeset=i + 1, installed_changeset=i) for i in range(10)]
|
||||
repositories = [create_repo(mock_app.install_model.context, changeset=i + 1, installed_changeset=i) for i in range(10)]
|
||||
mock_app.install_model.context.flush()
|
||||
return repositories
|
||||
|
||||
@@ -46,7 +46,7 @@ def tool_conf_repos(tool_shed_repository_cache):
|
||||
return tool_shed_repository_cache.local_repositories
|
||||
|
||||
|
||||
def create_repo(app, changeset, installed_changeset, config_filename=None):
|
||||
def create_repo(session, changeset, installed_changeset, config_filename=None):
|
||||
metadata = {
|
||||
'tools': [{
|
||||
'add_to_tool_panel': False, # to have repository.includes_tools_for_display_in_tool_panel=False in InstalledRepositoryManager.activate_repository()
|
||||
@@ -64,8 +64,8 @@ def create_repo(app, changeset, installed_changeset, config_filename=None):
|
||||
repository.installed_changeset_revision = str(installed_changeset)
|
||||
repository.deleted = False
|
||||
repository.uninstalled = False
|
||||
app.install_model.context.add(repository)
|
||||
app.install_model.context.flush()
|
||||
session.add(repository)
|
||||
session.flush()
|
||||
tool_dependency = tool_shed_install.ToolDependency(
|
||||
name='Name',
|
||||
version='100',
|
||||
@@ -73,5 +73,5 @@ def create_repo(app, changeset, installed_changeset, config_filename=None):
|
||||
status='ok',
|
||||
tool_shed_repository_id=repository.id,
|
||||
)
|
||||
app.install_model.context.add(tool_dependency)
|
||||
session.add(tool_dependency)
|
||||
return repository
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import pytest
|
||||
from sqlalchemy.orm.exc import DetachedInstanceError
|
||||
|
||||
from .conftest import create_repo
|
||||
|
||||
@@ -22,8 +21,8 @@ def test_add_repository_and_tool_conf_repository_to_repository_cache(tool_shed_r
|
||||
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.app, '21', '20')
|
||||
tool_shed_repository_cache.app.install_model.context.flush()
|
||||
create_repo(tool_shed_repository_cache.session, '21', '20')
|
||||
tool_shed_repository_cache.session.flush()
|
||||
tool_shed_repository_cache.rebuild()
|
||||
assert len(tool_shed_repository_cache.repositories) == 11
|
||||
assert len(tool_shed_repository_cache.local_repositories) == 10
|
||||
@@ -55,20 +54,3 @@ def test_get_installed_repository(tool_shed_repository_cache, repos, tool_conf_r
|
||||
assert repo
|
||||
else:
|
||||
assert repo is None
|
||||
|
||||
|
||||
def test_repo_cache_expunge(tool_shed_repository_cache, repos):
|
||||
tool_shed_repository_cache.rebuild()
|
||||
assert len(tool_shed_repository_cache.repositories) == 10
|
||||
# Modify and commit a repo, will expire in memory attributes of orm objects (unless using a different session)
|
||||
repo = tool_shed_repository_cache.repositories[0]
|
||||
repo.name = 'new name'
|
||||
tool_shed_repository_cache.app.install_model.session.flush()
|
||||
# remove session, should demonstrate the separate session is in use
|
||||
tool_shed_repository_cache.app.install_model.session.remove()
|
||||
assert repo.changeset_revision == "1"
|
||||
assert repo.tool_dependencies[0].name == 'Name'
|
||||
with pytest.raises(DetachedInstanceError):
|
||||
# Make sure this still raises DetachedInstanceError,
|
||||
# keeping this in memory would be expensive
|
||||
repo.metadata
|
||||
|
||||
Reference in New Issue
Block a user