From da61e77068f115ebe0afcd10a6d34b9dd43aaae7 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 13 Apr 2022 17:00:05 +0200 Subject: [PATCH] Fix persistent mulled cache unused for repo_has_name --- doc/source/admin/galaxy_options.rst | 14 +++- lib/galaxy/app.py | 7 +- lib/galaxy/config/sample/galaxy.yml.sample | 4 ++ lib/galaxy/config/schemas/config_schema.yml | 7 ++ .../deps/container_resolvers/__init__.py | 2 + .../deps/container_resolvers/mulled.py | 11 +++- lib/galaxy/tool_util/deps/mulled/util.py | 66 ++++++++++++------- packages/tool_util/test-requirements.txt | 1 + test/unit/tool_util/test_resolution_cache.py | 61 +++++++++++++++++ 9 files changed, 139 insertions(+), 34 deletions(-) create mode 100644 test/unit/tool_util/test_resolution_cache.py diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst index 7235cb45ab9..676b3e08fa3 100644 --- a/doc/source/admin/galaxy_options.rst +++ b/doc/source/admin/galaxy_options.rst @@ -1292,6 +1292,17 @@ :Type: str +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``mulled_resolution_cache_expire`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:Description: + Seconds until the beaker cache is considered old and a new value + is created. +:Default: ``3600`` +:Type: int + + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``object_store_config_file`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -4835,6 +4846,3 @@ . :Default: ``vault_conf.yml`` :Type: str - - - diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py index f80c7b131cf..31731dbfef9 100644 --- a/lib/galaxy/app.py +++ b/lib/galaxy/app.py @@ -218,9 +218,10 @@ class ConfiguresGalaxyMixin: mulled_resolution_cache = None if self.config.mulled_resolution_cache_type: cache_opts = { - 'cache.type': self.config.mulled_resolution_cache_type, - 'cache.data_dir': self.config.mulled_resolution_cache_data_dir, - 'cache.lock_dir': self.config.mulled_resolution_cache_lock_dir, + "cache.type": self.config.mulled_resolution_cache_type, + "cache.data_dir": self.config.mulled_resolution_cache_data_dir, + "cache.lock_dir": self.config.mulled_resolution_cache_lock_dir, + "cache.expire": self.config.mulled_resolution_cache_expire, } mulled_resolution_cache = CacheManager(**parse_cache_config_options(cache_opts)).get_cache('mulled_resolution') self.container_finder = containers.ContainerFinder(app_info, mulled_resolution_cache=mulled_resolution_cache) diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample index b64f2527938..7bff2d9b9b3 100644 --- a/lib/galaxy/config/sample/galaxy.yml.sample +++ b/lib/galaxy/config/sample/galaxy.yml.sample @@ -847,6 +847,10 @@ galaxy: # . #mulled_resolution_cache_lock_dir: mulled/locks + # Seconds until the beaker cache is considered old and a new value is + # created. + #mulled_resolution_cache_expire: 3600 + # Configuration file for the object store If this is set and exists, # it overrides any other objectstore settings. # The value of this option will be resolved with respect to diff --git a/lib/galaxy/config/schemas/config_schema.yml b/lib/galaxy/config/schemas/config_schema.yml index 28a61527bcd..cc4b3abad73 100644 --- a/lib/galaxy/config/schemas/config_schema.yml +++ b/lib/galaxy/config/schemas/config_schema.yml @@ -930,6 +930,13 @@ mapping: desc: | Lock directory used by beaker for caching mulled resolution requests. + mulled_resolution_cache_expire: + type: int + default: 3600 + required: false + desc: | + Seconds until the beaker cache is considered old and a new value is created. + object_store_config_file: type: str default: object_store_conf.xml diff --git a/lib/galaxy/tool_util/deps/container_resolvers/__init__.py b/lib/galaxy/tool_util/deps/container_resolvers/__init__.py index 654f6b9d994..5965b79f3ea 100644 --- a/lib/galaxy/tool_util/deps/container_resolvers/__init__.py +++ b/lib/galaxy/tool_util/deps/container_resolvers/__init__.py @@ -17,6 +17,8 @@ class ResolutionCache(Bunch): one resolution at a time in a single thread. """ + mulled_resolution_cache = None + class ContainerResolver(Dictifiable, metaclass=ABCMeta): """Description of a technique for resolving container images for tool execution.""" diff --git a/lib/galaxy/tool_util/deps/container_resolvers/mulled.py b/lib/galaxy/tool_util/deps/container_resolvers/mulled.py index bbae6793514..f92ad6aa053 100644 --- a/lib/galaxy/tool_util/deps/container_resolvers/mulled.py +++ b/lib/galaxy/tool_util/deps/container_resolvers/mulled.py @@ -19,6 +19,7 @@ from galaxy.util.commands import shell from ..container_classes import CONTAINER_CLASSES from ..container_resolvers import ( ContainerResolver, + ResolutionCache, ) from ..docker_util import build_docker_images_command from ..mulled.mulled_build import ( @@ -314,7 +315,9 @@ def singularity_cached_container_description(targets, cache_directory, hash_func return container -def targets_to_mulled_name(targets, hash_func, namespace, resolution_cache=None, session=None): +def targets_to_mulled_name( + targets, hash_func, namespace, resolution_cache: Optional[ResolutionCache] = None, session=None +): unresolved_cache_key = "galaxy.tool_util.deps.container_resolvers.mulled:unresolved" if resolution_cache is not None: if unresolved_cache_key not in resolution_cache: @@ -324,15 +327,17 @@ def targets_to_mulled_name(targets, hash_func, namespace, resolution_cache=None, unresolved_cache = set() mulled_resolution_cache = None - if resolution_cache and hasattr(resolution_cache, 'mulled_resolution_cache'): + if resolution_cache and resolution_cache.mulled_resolution_cache: mulled_resolution_cache = resolution_cache.mulled_resolution_cache name = None def cached_name(cache_key): if mulled_resolution_cache: - if cache_key in mulled_resolution_cache: + try: return resolution_cache.get(cache_key) + except KeyError: + return None return None if len(targets) == 1: diff --git a/lib/galaxy/tool_util/deps/mulled/util.py b/lib/galaxy/tool_util/deps/mulled/util.py index 307da3c49e7..7379cc5fc2f 100644 --- a/lib/galaxy/tool_util/deps/mulled/util.py +++ b/lib/galaxy/tool_util/deps/mulled/util.py @@ -18,6 +18,9 @@ QUAY_REPOSITORY_API_ENDPOINT = 'https://quay.io/api/v1/repository' BUILD_NUMBER_REGEX = re.compile(r'\d+$') PARSED_TAG = collections.namedtuple('PARSED_TAG', 'tag version build_string build_number') MULLED_SOCKET_TIMEOUT = 12 +QUAY_VERSIONS_CACHE_EXPIRY = 300 +NAMESPACE_HAS_REPO_NAME_KEY = "galaxy.tool_util.deps.container_resolvers.mulled.util:namespace_repo_names" +TAG_CACHE_KEY = "galaxy.tool_util.deps.container_resolvers.mulled.util:tag_cache" def create_repository(namespace, repo_name, oauth_token): @@ -60,29 +63,36 @@ def _namespace_has_repo_name(namespace, repo_name, resolution_cache): """ Get all quay containers in the biocontainers repo """ - cache_key = "galaxy.tool_util.deps.container_resolvers.mulled.util:namespace_repo_names" - if resolution_cache is not None and cache_key in resolution_cache: - repo_names = resolution_cache.get(cache_key) - else: - next_page = None - repo_names = [] - repos_headers = {"Accept-encoding": "gzip", "Accept": "application/json"} - while True: - repos_parameters = {"public": "true", "namespace": namespace, "next_page": next_page} - repos_response = requests.get( - QUAY_REPOSITORY_API_ENDPOINT, headers=repos_headers, params=repos_parameters, timeout=MULLED_SOCKET_TIMEOUT) - repos_response_json = repos_response.json() - repos = repos_response_json["repositories"] - repo_names += [r["name"] for r in repos] - next_page = repos_response_json.get("next_page") - if not next_page: - break - if resolution_cache is not None: - resolution_cache[cache_key] = repo_names + # resolution_cache.mulled_resolution_cache is the persistent variant of the resolution cache + resolution_cache = resolution_cache.mulled_resolution_cache or resolution_cache + cache_key = NAMESPACE_HAS_REPO_NAME_KEY + if resolution_cache is not None: + try: + return repo_name in resolution_cache.get(cache_key) + except KeyError: + pass + next_page = None + repo_names = [] + repos_headers = {"Accept-encoding": "gzip", "Accept": "application/json"} + while True: + repos_parameters = {"public": "true", "namespace": namespace, "next_page": next_page} + repos_response = requests.get( + QUAY_REPOSITORY_API_ENDPOINT, headers=repos_headers, params=repos_parameters, timeout=MULLED_SOCKET_TIMEOUT + ) + repos_response_json = repos_response.json() + repos = repos_response_json["repositories"] + repo_names += [r["name"] for r in repos] + next_page = repos_response_json.get("next_page") + if not next_page: + break + if resolution_cache is not None: + resolution_cache[cache_key] = repo_names return repo_name in repo_names -def mulled_tags_for(namespace, image, tag_prefix=None, resolution_cache=None, session=None): +def mulled_tags_for( + namespace, image, tag_prefix=None, resolution_cache=None, session=None, expire=QUAY_VERSIONS_CACHE_EXPIRY +): """Fetch remote tags available for supplied image name. The result will be sorted so newest tags are first. @@ -94,8 +104,13 @@ def mulled_tags_for(namespace, image, tag_prefix=None, resolution_cache=None, se log.info(f"skipping mulled_tags_for [{image}] no repository") return [] - cache_key = "galaxy.tool_util.deps.container_resolvers.mulled.util:tag_cache" + cache_key = TAG_CACHE_KEY if resolution_cache is not None: + if resolution_cache.mulled_resolution_cache is not None: + # Use persistent cache if possible. Since tags query is lightweight use a relatively short expiry time. + resolution_cache = resolution_cache.mulled_resolution_cache._get_cache( + "mulled_tag_cache", {"expire": expire} + ) if cache_key not in resolution_cache: resolution_cache[cache_key] = collections.defaultdict(dict) tag_cache = resolution_cache.get(cache_key) @@ -103,10 +118,11 @@ def mulled_tags_for(namespace, image, tag_prefix=None, resolution_cache=None, se tag_cache = collections.defaultdict(dict) tags_cached = False - if namespace in tag_cache: - if image in tag_cache[namespace]: - tags = tag_cache[namespace][image] - tags_cached = True + try: + tags = tag_cache[namespace][image] + tags_cached = True + except KeyError: + pass if not tags_cached: tags = quay_versions(namespace, image, session) diff --git a/packages/tool_util/test-requirements.txt b/packages/tool_util/test-requirements.txt index 12136494321..c32c10526b8 100644 --- a/packages/tool_util/test-requirements.txt +++ b/packages/tool_util/test-requirements.txt @@ -1,2 +1,3 @@ +beaker pytest pytest-mock diff --git a/test/unit/tool_util/test_resolution_cache.py b/test/unit/tool_util/test_resolution_cache.py new file mode 100644 index 00000000000..3ca5ae163e0 --- /dev/null +++ b/test/unit/tool_util/test_resolution_cache.py @@ -0,0 +1,61 @@ +import time + +import pytest +from beaker.cache import CacheManager +from beaker.util import parse_cache_config_options + +from galaxy.tool_util.deps.container_resolvers import ResolutionCache +from galaxy.tool_util.deps.container_resolvers.mulled import mulled_tags_for +from galaxy.tool_util.deps.mulled.util import ( + _namespace_has_repo_name, + NAMESPACE_HAS_REPO_NAME_KEY, + TAG_CACHE_KEY, +) + + +@pytest.fixture() +def resolution_cache(tmpdir): + resolution_cache = ResolutionCache() + cache_opts = { + "cache.type": "file", + "cache.data_dir": str(tmpdir / "data"), + "cache.lock_dir": str(tmpdir / "lock"), + "cache.expire": "1", + } + cm = CacheManager(**parse_cache_config_options(cache_opts)).get_cache( + "mulled_resolution" + ) + resolution_cache.mulled_resolution_cache = cm + return resolution_cache + + +def test_resolution_cache_namepace_has_repo_name(resolution_cache): + resolution_cache.mulled_resolution_cache[NAMESPACE_HAS_REPO_NAME_KEY] = [ + "mytool3000" + ] + assert _namespace_has_repo_name( + "bioconda", "mytool3000", resolution_cache=resolution_cache + ) + + +def test_resolution_cache_expires(resolution_cache): + resolution_cache.mulled_resolution_cache[NAMESPACE_HAS_REPO_NAME_KEY] = [ + "mytool3000" + ] + assert NAMESPACE_HAS_REPO_NAME_KEY in resolution_cache.mulled_resolution_cache + time.sleep(1.2) + assert NAMESPACE_HAS_REPO_NAME_KEY not in resolution_cache.mulled_resolution_cache + + +def test_targets_to_mulled_name(resolution_cache): + resolution_cache.mulled_resolution_cache[NAMESPACE_HAS_REPO_NAME_KEY] = [ + "mytool3000" + ] + cache = resolution_cache.mulled_resolution_cache._get_cache( + "mulled_tag_cache", {"expire": 1} + ) + cache[TAG_CACHE_KEY] = {"bioconda": {"mytool3000": ["1.0", "1.1"]}} + tags = mulled_tags_for( + namespace="bioconda", image="mytool3000", resolution_cache=resolution_cache + ) + assert tags == ["1.1", "1.0"]