From dbf7ac364a20835acf16ccdffea3018a58023e2d Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 10 Mar 2022 19:42:10 +0000 Subject: [PATCH 1/3] Check GitHub API rate limit in GitHubSearch. Deprecate `get_json()` search Follow-up on https://github.com/galaxyproject/galaxy/pull/13509 . --- .../tool_util/deps/mulled/mulled_search.py | 25 ++++++++++++++-- .../tool_util/mulled/test_mulled_search.py | 30 ++++++++----------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/lib/galaxy/tool_util/deps/mulled/mulled_search.py b/lib/galaxy/tool_util/deps/mulled/mulled_search.py index e2c1ccc6880..c521db9c7b5 100755 --- a/lib/galaxy/tool_util/deps/mulled/mulled_search.py +++ b/lib/galaxy/tool_util/deps/mulled/mulled_search.py @@ -5,6 +5,10 @@ import json import logging import sys import tempfile +from datetime import ( + datetime, + timezone, +) import requests @@ -143,14 +147,29 @@ class GitHubSearch: Tool to search the GitHub bioconda-recipes repo """ + @staticmethod + def _check_response_rate_limit(response): + if response.status_code == 403 and "API rate limit exceeded" in response.json()["message"]: + # It can take tens of minutes before the rate limit window resets + message = "GitHub API rate limit exceeded." + rate_limit_reset_UTC_timestamp = response.headers.get("X-RateLimit-Reset") + if rate_limit_reset_UTC_timestamp: + rate_limit_reset_datetime = datetime.fromtimestamp(int(rate_limit_reset_UTC_timestamp), tz=timezone.utc) + message += f" The rate limit window will reset at {rate_limit_reset_datetime.isoformat()}." + raise Exception(message) + def get_json(self, search_string): """ Takes search_string variable and return results from the bioconda-recipes github repository in JSON format + + DEPRECATED: this method is currently unreliable because the API query + sometimes succeeds but returns no items. """ response = requests.get( f"https://api.github.com/search/code?q={search_string}+in:path+repo:bioconda/bioconda-recipes+path:recipes", timeout=MULLED_SOCKET_TIMEOUT, ) + self._check_response_rate_limit(response) response.raise_for_status() return response.json() @@ -169,6 +188,7 @@ class GitHubSearch: f"https://api.github.com/repos/bioconda/bioconda-recipes/contents/recipes/{search_string}", timeout=MULLED_SOCKET_TIMEOUT, ) + self._check_response_rate_limit(response) return response.status_code == 200 @@ -392,10 +412,11 @@ def main(argv=None): github = GitHubSearch() for item in args.search: - github_json = github.get_json(item) - github_results[item] = github.process_json(github_json, item) if github.recipe_present(item): github_recipe_present.append(item) + else: + github_json = github.get_json(item) + github_results[item] = github.process_json(github_json, item) json_results["github"] = github_results json_results["github_recipe_present"] = {"recipes": github_recipe_present} diff --git a/test/unit/tool_util/mulled/test_mulled_search.py b/test/unit/tool_util/mulled/test_mulled_search.py index 20fa721538f..c74eee17275 100644 --- a/test/unit/tool_util/mulled/test_mulled_search.py +++ b/test/unit/tool_util/mulled/test_mulled_search.py @@ -1,5 +1,3 @@ -import time - import pytest from galaxy.tool_util.deps.mulled.mulled_search import ( @@ -36,23 +34,21 @@ def test_conda_search(): @external_dependency_management -def test_github_search(): +def test_github_recipe_present(): t = GitHubSearch() - search1 = t.process_json(t.get_json("adsfasdf"), "adsfasdf") - assert search1 == [] - - # The search sometimes returns no results so we retry a couple of times - num_retries = 5 - search2 = t.process_json(t.get_json("bioconductor-gosemsim"), "bioconductor-gosemsim") - while not search2 and num_retries: - num_retries -= 1 - time.sleep(2) # Wait a bit, otherwise, the search may fail because of throttling - search2 = t.process_json(t.get_json("bioconductor-gosemsim"), "bioconductor-gosemsim") - - assert search2 - for item in search2: - assert "bioconductor-gosemsim" in item["path"] + search_string2expected = { + "adsfasdf": False, + "bioconductor-gosemsim": True, + } + for search_string, expected in search_string2expected.items(): + try: + is_recipe_present = t.recipe_present(search_string) + except Exception as e: + if "API rate limit" in str(e): + pytest.skip("Hitting GitHub API rate limit") + raise + assert is_recipe_present is expected @external_dependency_management From dabc6f63890e74d347263d3fbb673720ff5ea84b Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Sun, 13 Mar 2022 20:22:25 +0000 Subject: [PATCH 2/3] Add `exec_search()` method to `CondaContext` Rework `best_search_result()` and `CondaInDockerContext` to use it. --- lib/galaxy/tool_util/deps/conda_util.py | 62 +++++++++---------- .../tool_util/deps/mulled/mulled_build.py | 38 ++++++++---- 2 files changed, 56 insertions(+), 44 deletions(-) diff --git a/lib/galaxy/tool_util/deps/conda_util.py b/lib/galaxy/tool_util/deps/conda_util.py index feb74a45fe6..2088241b88b 100644 --- a/lib/galaxy/tool_util/deps/conda_util.py +++ b/lib/galaxy/tool_util/deps/conda_util.py @@ -8,13 +8,15 @@ import shlex import shutil import sys import tempfile +from typing import List import packaging.version from galaxy.util import ( commands, + listify, smart_str, - unicodify, + which, ) from . import installable @@ -88,8 +90,8 @@ class CondaContext(installable.InstallableContext): ): self.condarc_override = condarc_override if not conda_exec and use_path_exec: - conda_exec = commands.which("conda") - if conda_exec: + conda_exec = which("conda") + if conda_exec and isinstance(conda_exec, str): conda_exec = os.path.normpath(conda_exec) self.conda_exec = conda_exec self.debug = debug @@ -179,8 +181,8 @@ class CondaContext(installable.InstallableContext): def conda_info(self): if self.conda_exec is not None: - info_out = commands.execute([self.conda_exec, "info", "--json"]) - info_out = unicodify(info_out) + cmd = listify(self.conda_exec) + ["info", "--json"] + info_out = commands.execute(cmd) info = json.loads(info_out) return info else: @@ -230,8 +232,7 @@ class CondaContext(installable.InstallableContext): Return the process exit code (i.e. 0 in case of success). """ - cmd = [self.conda_exec] - cmd.extend(operation.split()) + cmd = listify(self.conda_exec) + operation.split() if self.debug: cmd.append("--debug") cmd.extend(args) @@ -304,6 +305,24 @@ class CondaContext(installable.InstallableContext): stdout_path = "/dev/null" return self.exec_command("clean", clean_args, stdout_path=stdout_path) + def exec_search(self, args: List[str], json: bool = False, offline: bool = False, platform: str = None): + """ + Search conda channels for a package + + Return the standard output of the conda process. + """ + cmd = listify(self.conda_exec)[:] + cmd.append("search") + cmd.extend(self._override_channels_args) + if json: + cmd.append("--json") + if offline: + cmd.append("--offline") + if platform: + cmd.extend(["--platform", platform]) + cmd.extend(args) + return commands.execute(cmd) + def export_list(self, name, path): """ Return the process exit code (i.e. 0 in case of success). @@ -494,33 +513,14 @@ def cleanup_failed_install(conda_target, conda_context=None): cleanup_failed_install_of_environment(conda_target.install_environment, conda_context=conda_context) -def best_search_result(conda_target, conda_context, channels_override=None, offline=False, platform=None): +def best_search_result(conda_target, conda_context: CondaContext, offline: bool = False, platform: str = None): """Find best "conda search" result for specified target. Return ``None`` if no results match. """ - search_cmd = [] - conda_exec = conda_context.conda_exec - if isinstance(conda_exec, list): - # for CondaInDockerContext - search_cmd.extend(conda_exec) - else: - search_cmd.append(conda_exec) - search_cmd.extend(["search", "--full-name", "--json"]) - if offline: - search_cmd.append("--offline") - if platform: - search_cmd.extend(["--platform", platform]) - if channels_override: - search_cmd.append("--override-channels") - for channel in channels_override: - search_cmd.extend(["--channel", channel]) - else: - search_cmd.extend(conda_context._override_channels_args) - search_cmd.append(conda_target.package) + search_args = [conda_target.package] try: - res = commands.execute(search_cmd) - res = unicodify(res) + res = conda_context.exec_search(search_args, json=True, offline=offline, platform=platform) # Use python's stable list sorting to sort by date, # then build_number, then version. The top of the list # then is the newest version with the newest build and @@ -528,8 +528,8 @@ def best_search_result(conda_target, conda_context, channels_override=None, offl hits = json.loads(res).get(conda_target.package, [])[::-1] hits = sorted(hits, key=lambda hit: hit["build_number"], reverse=True) hits = sorted(hits, key=lambda hit: packaging.version.parse(hit["version"]), reverse=True) - except commands.CommandLineException: - log.error("Could not execute: '%s'", search_cmd) + except commands.CommandLineException as e: + log.error("Could not execute: '%s'\n%s", e.command, e) hits = [] if len(hits) == 0: diff --git a/lib/galaxy/tool_util/deps/mulled/mulled_build.py b/lib/galaxy/tool_util/deps/mulled/mulled_build.py index 054133fffe4..f424b555fe7 100644 --- a/lib/galaxy/tool_util/deps/mulled/mulled_build.py +++ b/lib/galaxy/tool_util/deps/mulled/mulled_build.py @@ -23,7 +23,10 @@ from sys import platform as _platform import yaml from galaxy.tool_util.deps import installable -from galaxy.tool_util.deps.conda_util import best_search_result +from galaxy.tool_util.deps.conda_util import ( + best_search_result, + CondaContext, +) from galaxy.tool_util.deps.docker_util import command_list as docker_command_list from galaxy.util import ( commands, @@ -326,18 +329,27 @@ def context_from_args(args): return InvolucroContext(involucro_bin=args.involucro_path, verbose=verbose) -class CondaInDockerContext: - @property - def conda_exec(self): - conda_image = CONDA_IMAGE or "continuumio/miniconda3:latest" - return docker_command_list("run", [conda_image, "conda"]) - - @property - def _override_channels_args(self): - override_channels_args = ["--override-channels"] - for channel in DEFAULT_CHANNELS: - override_channels_args.extend(["--channel", channel]) - return override_channels_args +class CondaInDockerContext(CondaContext): + def __init__( + self, + conda_prefix=None, + conda_exec=None, + shell_exec=None, + debug=False, + ensure_channels=DEFAULT_CHANNELS, + condarc_override=None, + ): + if not conda_exec: + conda_image = CONDA_IMAGE or "continuumio/miniconda3:latest" + conda_exec = docker_command_list("run", [conda_image, "conda"]) + super().__init__( + conda_prefix=conda_prefix, + conda_exec=conda_exec, + shell_exec=shell_exec, + debug=debug, + ensure_channels=ensure_channels, + condarc_override=condarc_override, + ) class InvolucroContext(installable.InstallableContext): From a6b1238be75496a18f71027d16c5221dd702c121 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Sun, 13 Mar 2022 20:27:54 +0000 Subject: [PATCH 3/3] Fix `CondaSearch` to use `conda` from PATH and `CondaContext.exec_search()` The `conda` package on PyPI is severely outdated and unmaintained, and the output of `run_command("search", ...)` did not parse correctly. Fix broken `test_conda_search` unit test. --- .../tool_util/deps/mulled/mulled_search.py | 24 +++++++++---------- .../tool_util/mulled/test_mulled_search.py | 8 +++---- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/lib/galaxy/tool_util/deps/mulled/mulled_search.py b/lib/galaxy/tool_util/deps/mulled/mulled_search.py index c521db9c7b5..3cd02de616d 100755 --- a/lib/galaxy/tool_util/deps/mulled/mulled_search.py +++ b/lib/galaxy/tool_util/deps/mulled/mulled_search.py @@ -12,6 +12,8 @@ from datetime import ( import requests +from galaxy.tool_util.deps.conda_util import CondaContext +from galaxy.util import which from .mulled_list import get_singularity_containers from .util import ( build_target, @@ -19,11 +21,6 @@ from .util import ( v2_image_name, ) -try: - from conda.cli.python_api import run_command -except ImportError: - run_command = None - try: from whoosh.fields import ( Schema, @@ -36,6 +33,7 @@ except ImportError: Schema = TEXT = STORED = create_in = QueryParser = None QUAY_API_URL = "https://quay.io/api/v1/repository" +conda_path = which("conda") class QuaySearch: @@ -131,11 +129,13 @@ class CondaSearch: Function takes search_string variable and returns results from the bioconda channel in JSON format """ - if run_command is None: - raise Exception(f"Invalid search destination. {deps_error_message('conda')}") - raw_out, err, exit_code = run_command("search", "-c", self.channel, search_string, use_exception_handler=True) - if exit_code != 0: - logging.info(f"Search failed with: {err}") + if not conda_path: + raise Exception("Invalid search destination. Required dependency [conda] is not in your PATH.") + try: + conda_context = CondaContext(conda_exec=conda_path, ensure_channels=self.channel) + raw_out = conda_context.exec_search([search_string]) + except Exception as e: + logging.info(f"Search failed with: {e}") return [] return [ {"package": n.split()[0], "version": n.split()[1], "build": n.split()[2]} for n in raw_out.split("\n")[2:-1] @@ -355,7 +355,7 @@ def main(argv=None): return destination_defaults = ["quay", "singularity", "github"] - if run_command is not None: + if conda_path: destination_defaults.append("conda") parser = argparse.ArgumentParser(description="Searches in a given quay organization for a repository") @@ -379,7 +379,7 @@ def main(argv=None): "--channel", dest="channel_string", default="bioconda", - help="Change conda channel to search; default is bioconda.", + help="Change conda channels to search; default is bioconda.", ) parser.add_argument( "--non-strict", diff --git a/test/unit/tool_util/mulled/test_mulled_search.py b/test/unit/tool_util/mulled/test_mulled_search.py index c74eee17275..451a2af9bfa 100644 --- a/test/unit/tool_util/mulled/test_mulled_search.py +++ b/test/unit/tool_util/mulled/test_mulled_search.py @@ -1,11 +1,11 @@ import pytest from galaxy.tool_util.deps.mulled.mulled_search import ( + conda_path, CondaSearch, get_package_hash, GitHubSearch, QuaySearch, - run_command, singularity_search, ) from ..util import external_dependency_management @@ -22,15 +22,13 @@ def test_quay_search(): @external_dependency_management -@pytest.mark.skipif(run_command is None, reason="requires import from conda library") +@pytest.mark.skipif(not conda_path, reason="requires conda on path") def test_conda_search(): t = CondaSearch("bioconda") search1 = t.get_json("asdfasdf") search2 = t.get_json("bioconductor-gosemsim") assert search1 == [] - assert search2["version"] == "2.2.0" - assert search2["package"] == "bioconductor-gosemsim" - assert search2["build"] == "0" + assert all(r["package"] == "bioconductor-gosemsim" for r in search2) @external_dependency_management