From 48e4a65bc626f58c39034221b8e2cad9e3439b0c Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Thu, 1 Feb 2024 15:22:56 +0100 Subject: [PATCH 01/24] mark abstract class as abstract --- conda_forge_tick/update_sources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda_forge_tick/update_sources.py b/conda_forge_tick/update_sources.py index 0d94f1845..9f29aa580 100644 --- a/conda_forge_tick/update_sources.py +++ b/conda_forge_tick/update_sources.py @@ -139,7 +139,7 @@ def get_url(self, meta_yaml) -> Optional[str]: pass -class VersionFromFeed(AbstractSource): +class VersionFromFeed(AbstractSource, abc.ABC): name = "VersionFromFeed" ver_prefix_remove = ["release-", "releases%2F", "v_", "v.", "v"] dev_vers = [ From 452f182a0e3cfb7f500d81ea7bcf3e64a48e79d8 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Thu, 8 Feb 2024 17:46:17 +0100 Subject: [PATCH 02/24] add online JSON backend --- conda_forge_tick/backend_settings.py | 3 + conda_forge_tick/cli.py | 22 ++- conda_forge_tick/lazy_json_backends.py | 209 ++++++++++++++++++++----- tests/test_lazy_json_backends.py | 44 ++++++ 4 files changed, 235 insertions(+), 43 deletions(-) create mode 100644 conda_forge_tick/backend_settings.py diff --git a/conda_forge_tick/backend_settings.py b/conda_forge_tick/backend_settings.py new file mode 100644 index 000000000..538b7bda2 --- /dev/null +++ b/conda_forge_tick/backend_settings.py @@ -0,0 +1,3 @@ +GITHUB_GRAPH_BACKEND_BASE_URL = ( + "https://github.com/regro/cf-graph-countyfair/raw/master" +) diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index ca34b2f77..704ba6e40 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -1,3 +1,4 @@ +import logging import os import time from typing import Optional @@ -5,8 +6,13 @@ import click from click import IntRange +from conda_forge_tick import lazy_json_backends +from conda_forge_tick.utils import setup_logger + from .cli_context import CliContext +logger = logging.getLogger(__name__) + pass_context = click.make_pass_decorator(CliContext, ensure=True) job_option = click.option( "--job", @@ -46,14 +52,28 @@ def invoke(self, ctx: click.Context): default=False, help="dry run: don't push changes to PRs or graph to Github", ) +@click.option( + "--online/--offline", + default=False, + help="Online: Use the GitHub API for accessing the dependency graph. This is useful for local testing. Note " + "however that any write operations will not be performed.", +) @pass_context -def main(ctx: CliContext, debug: bool, dry_run: bool) -> None: +def main(ctx: CliContext, debug: bool, dry_run: bool, online: bool) -> None: + log_level = "debug" if debug else "info" + setup_logger(logger, log_level) + ctx.debug = debug ctx.dry_run = dry_run if ctx.debug: os.environ["CONDA_FORGE_TICK_DEBUG"] = "1" + if online: + logger.info("Running in online mode") + lazy_json_backends.CF_TICK_GRAPH_DATA_BACKENDS = ("github",) + lazy_json_backends.CF_TICK_GRAPH_DATA_PRIMARY_BACKEND = "github" + @main.command(name="gather-all-feedstocks") @pass_context diff --git a/conda_forge_tick/lazy_json_backends.py b/conda_forge_tick/lazy_json_backends.py index 1ac9153b7..e0610cf5a 100644 --- a/conda_forge_tick/lazy_json_backends.py +++ b/conda_forge_tick/lazy_json_backends.py @@ -5,14 +5,29 @@ import logging import os import subprocess +import urllib +from abc import ABC, abstractmethod from collections.abc import Callable, MutableMapping -from typing import IO, Any, Iterator, Optional, Set, Union +from typing import ( + IO, + Any, + Dict, + Iterable, + Iterator, + List, + Mapping, + Optional, + Set, + Union, +) import rapidjson as json +import requests +from .backend_settings import GITHUB_GRAPH_BACKEND_BASE_URL from .cli_context import CliContext -logger = logging.getLogger("conda_forge_tick.lazy_json_backends") +logger = logging.getLogger(__name__) CF_TICK_GRAPH_DATA_BACKENDS = tuple( os.environ.get("CF_TICK_GRAPH_DATA_BACKENDS", "file").split(":"), @@ -40,80 +55,86 @@ def get_sharded_path(file_path, n_dirs=5): return os.path.join(*pth_parts) -class LazyJsonBackend: +class LazyJsonBackend(ABC): @contextlib.contextmanager - def transaction_context(self): - raise NotImplementedError + @abstractmethod + def transaction_context(self) -> "LazyJsonBackend": + pass @contextlib.contextmanager - def snapshot_context(self): - raise NotImplementedError + @abstractmethod + def snapshot_context(self) -> "LazyJsonBackend": + pass - def hexists(self, name, key): - raise NotImplementedError + @abstractmethod + def hexists(self, name: str, key: str) -> bool: + pass - def hset(self, name, key, value): - raise NotImplementedError + @abstractmethod + def hset(self, name: str, key: str, value: str) -> None: + pass - def hmset(self, name, mapping): - raise NotImplementedError + @abstractmethod + def hmset(self, name: str, mapping: Mapping[str, str]) -> None: + pass - def hmget(self, name, keys): - raise NotImplementedError + @abstractmethod + def hmget(self, name: str, keys: Iterable[str]) -> List[str]: + pass - def hdel(self, name, keys): - raise NotImplementedError + @abstractmethod + def hdel(self, name: str, keys: Iterable[str]) -> None: + pass - def hkeys(self, name): - raise NotImplementedError + @abstractmethod + def hkeys(self, name: str) -> List[str]: + pass - def hsetnx(self, name, key, value): + def hsetnx(self, name: str, key: str, value: str) -> bool: if self.hexists(name, key): return False else: self.hset(name, key, value) return True - def hget(self, name, key): - raise NotImplementedError + @abstractmethod + def hget(self, name: str, key: str) -> str: + pass - def hgetall(self, name, hashval=False): - raise NotImplementedError + @abstractmethod + def hgetall(self, name: str, hashval: bool = False) -> Dict[str, str]: + pass class FileLazyJsonBackend(LazyJsonBackend): @contextlib.contextmanager - def transaction_context(self): - try: - yield self - finally: - pass + def transaction_context(self) -> "FileLazyJsonBackend": + # context not required + yield self @contextlib.contextmanager - def snapshot_context(self): - try: - yield self - finally: - pass + def snapshot_context(self) -> "FileLazyJsonBackend": + # context not required + yield self - def hexists(self, name, key): + def hexists(self, name: str, key: str) -> bool: return os.path.exists(get_sharded_path(f"{name}/{key}.json")) - def hset(self, name, key, value): + def hset(self, name: str, key: str, value: str) -> None: sharded_path = get_sharded_path(f"{name}/{key}.json") if os.path.split(sharded_path)[0]: os.makedirs(os.path.split(sharded_path)[0], exist_ok=True) with open(sharded_path, "w") as f: f.write(value) - def hmset(self, name, mapping): + def hmset(self, name: str, mapping: Mapping[str, str]) -> None: for key, value in mapping.items(): self.hset(name, key, value) - def hmget(self, name, keys): + def hmget(self, name: str, keys: str) -> List[str]: return [self.hget(name, key) for key in keys] - def hgetall(self, name, hashval=False): + def hgetall(self, name: str, hashval: bool = False) -> Dict[str, str]: return { key: ( hashlib.sha256(self.hget(name, key).encode("utf-8")).hexdigest() @@ -123,7 +144,7 @@ def hgetall(self, name, hashval=False): for key in self.hkeys(name) } - def hdel(self, name, keys): + def hdel(self, name: str, keys: Iterable[str]) -> None: from .executors import DLOCK, PRLOCK, TRLOCK lzj_names = " ".join(get_sharded_path(f"{name}/{key}.json") for key in keys) @@ -139,7 +160,7 @@ def hdel(self, name, keys): capture_output=True, ) - def hkeys(self, name): + def hkeys(self, name: str) -> List[str]: jlen = len(".json") if name == "lazy_json": fnames = glob.glob("*.json") @@ -151,13 +172,116 @@ def hkeys(self, name): fnames = glob.glob(os.path.join(name, "**/*.json"), recursive=True) return [os.path.basename(fname)[:-jlen] for fname in fnames] - def hget(self, name, key): + def hget(self, name: str, key: str) -> str: sharded_path = get_sharded_path(f"{name}/{key}.json") with open(sharded_path) as f: data_str = f.read() return data_str +class GithubLazyJsonBackend(LazyJsonBackend): + """ + Read-only backend that makes live requests to https://raw.githubusercontent.com URLs for serving + JSON files. + + Any write operations are ignored! + """ + + _write_warned = False + _n_requests = 0 + + def __init__(self) -> None: + self.base_url = GITHUB_GRAPH_BACKEND_BASE_URL + + @property + def base_url(self) -> str: + return self._base_url + + @base_url.setter + def base_url(self, value: str) -> None: + if not value.endswith("/"): + value += "/" + self._base_url = value + + def _ignore_write(self) -> None: + if self._write_warned: + return + logger.info(f"Note: Write operations to the GitHub online backend are ignored.") + self._write_warned = True + + @classmethod + def _inform_web_request(cls): + cls._n_requests += 1 + if cls._n_requests % 10 == 0: + logger.info( + f"Made {cls._n_requests} requests to the GitHub online backend.", + ) + if cls._n_requests == 10: + logger.warning( + f"Using the GitHub online backend for making a lot of requests is not recommended.", + ) + + def transaction_context(self) -> "GithubLazyJsonBackend": + # context not required + yield self + + def snapshot_context(self) -> "GithubLazyJsonBackend": + # context not required + yield self + + def hexists(self, name: str, key: str) -> bool: + self._inform_web_request() + url = urllib.parse.urljoin(self.base_url, f"{name}/{key}.json") + status = requests.head(url).status_code + + if status == 200: + return True + if status == 404: + return False + + raise RuntimeError(f"Unexpected status code {status} for {url}") + + def hset(self, name: str, key: str, value: str) -> None: + self._ignore_write() + + def hmset(self, name: str, mapping: Mapping[str, str]) -> None: + self._ignore_write() + + def hmget(self, name: str, keys: Iterable[str]) -> List[str]: + return [self.hget(name, key) for key in keys] + + def hdel(self, name: str, keys: Iterable[str]) -> None: + self._ignore_write() + + def hkeys(self, name: str) -> List[str]: + """ + Not implemented for GithubLazyJsonBackend. + Logs a warning and returns an empty list. + """ + logger.warning( + "hkeys not implemented for GithubLazyJsonBackend. Returning empty list.", + ) + return [] + + def hget(self, name: str, key: str) -> str: + self._inform_web_request() + sharded_path = get_sharded_path(f"{name}/{key}.json") + url = urllib.parse.urljoin(self.base_url, sharded_path) + r = requests.get(url) + r.raise_for_status() + return r.text + + def hgetall(self, name: str, hashval: bool = False) -> Dict[str, str]: + """ + Not implemented for GithubLazyJsonBackend. + Logs a warning and returns an empty dict. + """ + logger.warning( + "hgetall not implemented for GithubLazyJsonBackend. Returning empty dict.", + ) + return {} + + @functools.lru_cache(maxsize=128) def _get_graph_data_mongodb_client_cached(pid): import pymongo @@ -313,6 +437,7 @@ def hget(self, name, key): LAZY_JSON_BACKENDS = { "file": FileLazyJsonBackend, "mongodb": MongoDBLazyJsonBackend, + "github": GithubLazyJsonBackend, } diff --git a/tests/test_lazy_json_backends.py b/tests/test_lazy_json_backends.py index ad66ec89d..b16499b05 100644 --- a/tests/test_lazy_json_backends.py +++ b/tests/test_lazy_json_backends.py @@ -2,12 +2,16 @@ import json import os import pickle +from unittest import mock +from unittest.mock import MagicMock import pytest import conda_forge_tick.utils +from conda_forge_tick.backend_settings import GITHUB_GRAPH_BACKEND_BASE_URL from conda_forge_tick.lazy_json_backends import ( LAZY_JSON_BACKENDS, + GithubLazyJsonBackend, LazyJson, MongoDBLazyJsonBackend, dump, @@ -506,3 +510,43 @@ def test_lazy_json_backends_hashmap(tmpdir): assert get_all_keys_for_hashmap("lazy_json") == ["blah"] remove_key_for_hashmap("lazy_json", "blah") assert get_all_keys_for_hashmap("lazy_json") == [] + + +class TestGithubLazyJsonBackend: + base_url = "https://github.com/lorem/ipsum/raw/master" + + @pytest.fixture + def backend(self) -> GithubLazyJsonBackend: + github_backend = GithubLazyJsonBackend() + github_backend.base_url = self.base_url + return github_backend + + def test_base_url(self) -> None: + github_backend = GithubLazyJsonBackend() + assert github_backend.base_url == GITHUB_GRAPH_BACKEND_BASE_URL + "/" + github_backend.base_url = self.base_url + assert github_backend.base_url == self.base_url + "/" + + @mock.patch("requests.head") + def test_hexists_success( + self, + mock_head: MagicMock, + backend: GithubLazyJsonBackend, + ) -> None: + mock_head.return_value.status_code = 200 + assert backend.hexists("name", "key") + mock_head.assert_called_once_with( + f"{TestGithubLazyJsonBackend.base_url}/name/key.json", + ) + + @mock.patch("requests.head") + def test_hexists_failure( + self, + mock_head: MagicMock, + backend: GithubLazyJsonBackend, + ) -> None: + mock_head.return_value.status_code = 404 + assert not backend.hexists("name", "key") + mock_head.assert_called_once_with( + f"{TestGithubLazyJsonBackend.base_url}/name/key.json", + ) From ce70ef1bac9854b065bbf777d171586f60eba7cb Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Thu, 8 Feb 2024 17:57:55 +0100 Subject: [PATCH 03/24] use logging consistently, configure centrally --- conda_forge_tick/all_feedstocks.py | 12 +++------- conda_forge_tick/auto_tick.py | 21 ++++++---------- conda_forge_tick/cli.py | 5 ++-- conda_forge_tick/depfinder_api.py | 2 +- conda_forge_tick/executors.py | 2 +- conda_forge_tick/feedstock_parser.py | 2 +- conda_forge_tick/lazy_json_backends.py | 25 +++++--------------- conda_forge_tick/lazy_json_backups.py | 9 +------ conda_forge_tick/make_graph.py | 10 +++----- conda_forge_tick/migrators/core.py | 2 +- conda_forge_tick/migrators/cross_compile.py | 2 +- conda_forge_tick/migrators/dep_updates.py | 2 +- conda_forge_tick/migrators/license.py | 2 +- conda_forge_tick/migrators/migration_yaml.py | 10 ++++---- conda_forge_tick/migrators/version.py | 2 +- conda_forge_tick/update_deps.py | 2 +- conda_forge_tick/update_prs.py | 9 ++----- conda_forge_tick/update_recipe/version.py | 2 +- conda_forge_tick/update_sources.py | 2 +- conda_forge_tick/update_upstream_versions.py | 7 +----- conda_forge_tick/utils.py | 17 +++++-------- 21 files changed, 48 insertions(+), 99 deletions(-) diff --git a/conda_forge_tick/all_feedstocks.py b/conda_forge_tick/all_feedstocks.py index e16666512..0f55cf179 100644 --- a/conda_forge_tick/all_feedstocks.py +++ b/conda_forge_tick/all_feedstocks.py @@ -1,16 +1,14 @@ import logging -from typing import Any, List +from typing import List import github import tqdm from conda_forge_tick import sensitive_env -from .cli_context import CliContext from .lazy_json_backends import dump, load -from .utils import setup_logger -logger = logging.getLogger("conda_forge_tick.all_feedstocks") +logger = logging.getLogger(__name__) def get_all_feedstocks_from_github(): @@ -67,11 +65,7 @@ def get_archived_feedstocks(cached: bool = False) -> List[str]: return names -def main(ctx: CliContext) -> None: - if ctx.debug: - setup_logger(logger, level="debug") - else: - setup_logger(logger) +def main() -> None: logger.info("fetching active feedstocks from github") data = get_all_feedstocks_from_github() with open("all_feedstocks.json", "w") as fp: diff --git a/conda_forge_tick/auto_tick.py b/conda_forge_tick/auto_tick.py index be47cb8ee..de0253b31 100644 --- a/conda_forge_tick/auto_tick.py +++ b/conda_forge_tick/auto_tick.py @@ -27,6 +27,12 @@ import tqdm from .cli_context import CliContext +from .lazy_json_backends import ( + LazyJson, + get_all_keys_for_hashmap, + lazy_json_transaction, + remove_key_for_hashmap, +) if typing.TYPE_CHECKING: from .migrators_types import MetaYamlTypedDict, MigrationUidTypedDict, PackageName @@ -54,12 +60,6 @@ is_github_api_limit_reached, push_repo, ) -from conda_forge_tick.lazy_json_backends import ( - LazyJson, - get_all_keys_for_hashmap, - lazy_json_transaction, - remove_key_for_hashmap, -) from conda_forge_tick.migrators import ( ArchRebuild, Build2HostMigrator, @@ -105,14 +105,13 @@ parse_munged_run_export, pluck, sanitize_string, - setup_logger, yaml_safe_load, ) # not using this right now # from conda_forge_tick.deploy import deploy -logger = logging.getLogger("conda_forge_tick.auto_tick") +logger = logging.getLogger(__name__) PR_LIMIT = 5 MAX_PR_LIMIT = 50 @@ -1566,12 +1565,6 @@ def main(ctx: CliContext) -> None: global BOT_HOME_DIR BOT_HOME_DIR = os.getcwd() - # logging - if ctx.debug: - setup_logger(logging.getLogger("conda_forge_tick"), level="debug") - else: - setup_logger(logging.getLogger("conda_forge_tick")) - with fold_log_lines("updating graph with PR info"): _update_graph_with_pr_info() diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index 704ba6e40..ff1146d92 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -7,7 +7,6 @@ from click import IntRange from conda_forge_tick import lazy_json_backends -from conda_forge_tick.utils import setup_logger from .cli_context import CliContext @@ -61,7 +60,7 @@ def invoke(self, ctx: click.Context): @pass_context def main(ctx: CliContext, debug: bool, dry_run: bool, online: bool) -> None: log_level = "debug" if debug else "info" - setup_logger(logger, log_level) + setup_logging(log_level) ctx.debug = debug ctx.dry_run = dry_run @@ -80,7 +79,7 @@ def main(ctx: CliContext, debug: bool, dry_run: bool, online: bool) -> None: def gather_all_feedstocks(ctx: CliContext) -> None: from . import all_feedstocks - all_feedstocks.main(ctx) + all_feedstocks.main() @main.command(name="make-graph") diff --git a/conda_forge_tick/depfinder_api.py b/conda_forge_tick/depfinder_api.py index 72908d397..01f4356d5 100644 --- a/conda_forge_tick/depfinder_api.py +++ b/conda_forge_tick/depfinder_api.py @@ -40,7 +40,7 @@ from depfinder.stdliblist import builtin_modules as _builtin_modules from depfinder.utils import SKETCHY_TYPES_TABLE -logger = logging.getLogger("conda_forge_tick.depfinder_api") +logger = logging.getLogger(__name__) def extract_pkg_from_import(name): diff --git a/conda_forge_tick/executors.py b/conda_forge_tick/executors.py index 0f2448a4f..83bd71e9f 100644 --- a/conda_forge_tick/executors.py +++ b/conda_forge_tick/executors.py @@ -19,7 +19,7 @@ def __exit__(self, *args, **kwargs): DLOCK = DummyLock() -logger = logging.getLogger("conda_forge_tick.executor") +logger = logging.getLogger(__name__) def _init_process(lock): diff --git a/conda_forge_tick/feedstock_parser.py b/conda_forge_tick/feedstock_parser.py index 9753579ba..f798c80ed 100644 --- a/conda_forge_tick/feedstock_parser.py +++ b/conda_forge_tick/feedstock_parser.py @@ -22,7 +22,7 @@ from .utils import as_iterable, parse_meta_yaml -logger = logging.getLogger("conda_forge_tick.feedstock_parser") +logger = logging.getLogger(__name__) PIN_SEP_PAT = re.compile(r" |>|<|=|\[") diff --git a/conda_forge_tick/lazy_json_backends.py b/conda_forge_tick/lazy_json_backends.py index e0610cf5a..c637cf823 100644 --- a/conda_forge_tick/lazy_json_backends.py +++ b/conda_forge_tick/lazy_json_backends.py @@ -203,20 +203,21 @@ def base_url(self, value: str) -> None: value += "/" self._base_url = value - def _ignore_write(self) -> None: - if self._write_warned: + @classmethod + def _ignore_write(cls) -> None: + if cls._write_warned: return logger.info(f"Note: Write operations to the GitHub online backend are ignored.") - self._write_warned = True + cls._write_warned = True @classmethod def _inform_web_request(cls): cls._n_requests += 1 - if cls._n_requests % 10 == 0: + if cls._n_requests % 20 == 0: logger.info( f"Made {cls._n_requests} requests to the GitHub online backend.", ) - if cls._n_requests == 10: + if cls._n_requests == 20: logger.warning( f"Using the GitHub online backend for making a lot of requests is not recommended.", ) @@ -855,27 +856,13 @@ def load( def main_sync(ctx: CliContext): - from conda_forge_tick.utils import setup_logger - - if ctx.debug: - setup_logger(logging.getLogger("conda_forge_tick"), level="debug") - else: - setup_logger(logging.getLogger("conda_forge_tick")) - if not ctx.dry_run: sync_lazy_json_across_backends() def main_cache(ctx: CliContext): - from conda_forge_tick.utils import setup_logger - global CF_TICK_GRAPH_DATA_BACKENDS - if ctx.debug: - setup_logger(logging.getLogger("conda_forge_tick"), level="debug") - else: - setup_logger(logging.getLogger("conda_forge_tick")) - if not ctx.dry_run and len(CF_TICK_GRAPH_DATA_BACKENDS) > 1: OLD_CF_TICK_GRAPH_DATA_BACKENDS = CF_TICK_GRAPH_DATA_BACKENDS try: diff --git a/conda_forge_tick/lazy_json_backups.py b/conda_forge_tick/lazy_json_backups.py index bc6ac5f3e..23ee722f8 100644 --- a/conda_forge_tick/lazy_json_backups.py +++ b/conda_forge_tick/lazy_json_backups.py @@ -14,7 +14,7 @@ from .cli_context import CliContext -logger = logging.getLogger("conda_forge_tick.lazy_json_backends") +logger = logging.getLogger(__name__) CF_TICK_GRAPH_DATA_BACKUP_BACKEND = os.environ.get( "CF_TICK_GRAPH_DATA_BACKUP_BACKEND", @@ -164,13 +164,6 @@ def save_backup(fname): def main_backup(ctx: CliContext): - from conda_forge_tick.utils import setup_logger - - if ctx.debug: - setup_logger(logging.getLogger("conda_forge_tick"), level="debug") - else: - setup_logger(logging.getLogger("conda_forge_tick")) - def _name_to_ts(b): return int(b.split(".")[0].split("_")[-1]) diff --git a/conda_forge_tick/make_graph.py b/conda_forge_tick/make_graph.py index 52f029718..5af44d51a 100644 --- a/conda_forge_tick/make_graph.py +++ b/conda_forge_tick/make_graph.py @@ -26,12 +26,13 @@ from .cli_context import CliContext from .contexts import GithubContext from .executors import executor -from .utils import as_iterable, dump_graph, load_graph, setup_logger +from .utils import as_iterable, dump_graph, load_graph # from conda_forge_tick.profiler import profiling -logger = logging.getLogger("conda_forge_tick.make_graph") +logger = logging.getLogger(__name__) + pin_sep_pat = re.compile(r" |>|<|=|\[") random.seed(os.urandom(64)) @@ -312,11 +313,6 @@ def _migrate_schemas(): # @profiling def main(ctx: CliContext) -> None: - if ctx.debug: - setup_logger(logging.getLogger("conda_forge_tick"), level="debug") - else: - setup_logger(logging.getLogger("conda_forge_tick")) - names = get_all_feedstocks(cached=True) gx = load_graph() diff --git a/conda_forge_tick/migrators/core.py b/conda_forge_tick/migrators/core.py index 31c0da763..b678da1b8 100644 --- a/conda_forge_tick/migrators/core.py +++ b/conda_forge_tick/migrators/core.py @@ -24,7 +24,7 @@ from ..migrators_types import AttrsTypedDict, MigrationUidTypedDict, PackageName -logger = logging.getLogger("conda_forge_tick.migrators.core") +logger = logging.getLogger(__name__) def _sanitized_muids(pred: List[dict]) -> List["JsonFriendly"]: diff --git a/conda_forge_tick/migrators/cross_compile.py b/conda_forge_tick/migrators/cross_compile.py index aed6ad2b6..e71bf673a 100644 --- a/conda_forge_tick/migrators/cross_compile.py +++ b/conda_forge_tick/migrators/cross_compile.py @@ -10,7 +10,7 @@ if typing.TYPE_CHECKING: from ..migrators_types import AttrsTypedDict -logger = logging.getLogger("conda_forge_tick.migrators.cross_compile") +logger = logging.getLogger(__name__) class CrossCompilationMigratorBase(MiniMigrator): diff --git a/conda_forge_tick/migrators/dep_updates.py b/conda_forge_tick/migrators/dep_updates.py index 4cc819f56..479a26807 100644 --- a/conda_forge_tick/migrators/dep_updates.py +++ b/conda_forge_tick/migrators/dep_updates.py @@ -9,7 +9,7 @@ if typing.TYPE_CHECKING: from ..migrators_types import AttrsTypedDict -logger = logging.getLogger("conda_forge_tick.migrators.dep_updates") +logger = logging.getLogger(__name__) class DependencyUpdateMigrator(MiniMigrator): diff --git a/conda_forge_tick/migrators/license.py b/conda_forge_tick/migrators/license.py index d45fe6821..d3adc1c38 100644 --- a/conda_forge_tick/migrators/license.py +++ b/conda_forge_tick/migrators/license.py @@ -23,7 +23,7 @@ LICENSE_SPLIT = re.compile(r"\||\+") -logger = logging.getLogger("conda_forge_tick.migrators.license") +logger = logging.getLogger(__name__) def _to_spdx(lic): diff --git a/conda_forge_tick/migrators/migration_yaml.py b/conda_forge_tick/migrators/migration_yaml.py index fd786a9ec..58e21aaab 100644 --- a/conda_forge_tick/migrators/migration_yaml.py +++ b/conda_forge_tick/migrators/migration_yaml.py @@ -26,7 +26,7 @@ if typing.TYPE_CHECKING: from ..migrators_types import AttrsTypedDict, MigrationUidTypedDict, PackageName -logger = logging.getLogger("conda_forge_tick.migrators.migration_yaml") +logger = logging.getLogger(__name__) def _patch_dict(cfg, patches): @@ -403,9 +403,11 @@ def _not_has_error(node): graph, key=lambda x: ( _not_has_error(x), - random.uniform(0, 1) - if not _not_has_error(x) - else len(nx.descendants(total_graph, x)), + ( + random.uniform(0, 1) + if not _not_has_error(x) + else len(nx.descendants(total_graph, x)) + ), x, ), reverse=True, diff --git a/conda_forge_tick/migrators/version.py b/conda_forge_tick/migrators/version.py index 26446b1ac..98e8a6338 100644 --- a/conda_forge_tick/migrators/version.py +++ b/conda_forge_tick/migrators/version.py @@ -28,7 +28,7 @@ "ansible", ] -logger = logging.getLogger("conda_forge_tick.migrators.version") +logger = logging.getLogger(__name__) def _fmt_error_message(errors, version): diff --git a/conda_forge_tick/update_deps.py b/conda_forge_tick/update_deps.py index 60158b46e..7e5c1c793 100644 --- a/conda_forge_tick/update_deps.py +++ b/conda_forge_tick/update_deps.py @@ -22,7 +22,7 @@ except ImportError: from grayskull.__main__ import create_python_recipe -logger = logging.getLogger("conda_forge_tick.update_deps") +logger = logging.getLogger(__name__) SECTIONS_TO_PARSE = ["host", "run"] diff --git a/conda_forge_tick/update_prs.py b/conda_forge_tick/update_prs.py index 02812ded1..9a03d2361 100644 --- a/conda_forge_tick/update_prs.py +++ b/conda_forge_tick/update_prs.py @@ -19,11 +19,11 @@ from .executors import executor from .make_graph import ghctx -from .utils import github_client, load_graph, setup_logger +from .utils import github_client, load_graph # from conda_forge_tick.profiler import profiling -logger = logging.getLogger("conda_forge_tick.update_prs") +logger = logging.getLogger(__name__) NUM_GITHUB_THREADS = 2 KEEP_PR_FRACTION = 1.5 @@ -165,11 +165,6 @@ def close_dirty_prs( # @profiling def main(ctx: CliContext, job: int = 1, n_jobs: int = 1) -> None: - if ctx.debug: - setup_logger(logger, level="debug") - else: - setup_logger(logger) - gx = load_graph() gx = close_labels(gx, ctx.dry_run, job=job, n_jobs=n_jobs) diff --git a/conda_forge_tick/update_recipe/version.py b/conda_forge_tick/update_recipe/version.py index f3ce4522f..73e252577 100644 --- a/conda_forge_tick/update_recipe/version.py +++ b/conda_forge_tick/update_recipe/version.py @@ -25,7 +25,7 @@ # matches valid jinja2 vars JINJA2_VAR_RE = re.compile("{{ ((?:[a-zA-Z]|(?:_[a-zA-Z0-9]))[a-zA-Z0-9_]*) }}") -logger = logging.getLogger("conda_forge_tick.update_recipe.version") +logger = logging.getLogger(__name__) def _gen_key_selector(dct: MutableMapping, key: str): diff --git a/conda_forge_tick/update_sources.py b/conda_forge_tick/update_sources.py index 9f29aa580..8258d53b5 100644 --- a/conda_forge_tick/update_sources.py +++ b/conda_forge_tick/update_sources.py @@ -27,7 +27,7 @@ CRAN_INDEX: Optional[dict] = None -logger = logging.getLogger("conda_forge_tick._update_version.update_sources") +logger = logging.getLogger(__name__) CURL_ONLY_URL_SLUGS = [ "https://eups.lsst.codes/", diff --git a/conda_forge_tick/update_upstream_versions.py b/conda_forge_tick/update_upstream_versions.py index 59e046162..53abf58a5 100644 --- a/conda_forge_tick/update_upstream_versions.py +++ b/conda_forge_tick/update_upstream_versions.py @@ -36,7 +36,7 @@ RawURL, ROSDistro, ) -from .utils import get_keys_default, load_graph, setup_logger +from .utils import get_keys_default, load_graph T = TypeVar("T") @@ -403,11 +403,6 @@ def main( :param n_jobs: The total number of jobs. :param package: The package to update. If None, update all packages. """ - if ctx.debug: - setup_logger(logger, level="debug") - else: - setup_logger(logger) - logger.info("Reading graph") # Graph enabled for inspection gx = load_graph() diff --git a/conda_forge_tick/utils.py b/conda_forge_tick/utils.py index 6db8babbf..a13df82f7 100644 --- a/conda_forge_tick/utils.py +++ b/conda_forge_tick/utils.py @@ -27,7 +27,7 @@ from conda_forge_tick.migrators_types import MetaYamlTypedDict -logger = logging.getLogger("conda_forge_tick.utils") +logger = logging.getLogger(__name__) T = typing.TypeVar("T") TD = typing.TypeVar("TD", bound=dict, covariant=True) @@ -394,17 +394,12 @@ def __getitem__(self, name: Any) -> str: return f'{self}["{name}"]' -def setup_logger(logger: logging.Logger, level: str = "INFO") -> None: - """Basic configuration for logging""" - logger.setLevel(level.upper()) - ch = logging.StreamHandler() - ch.setLevel(level.upper()) - ch.setFormatter( - logging.Formatter("%(asctime)-15s %(levelname)-8s %(name)s || %(message)s"), +def setup_logging(level: str = "INFO") -> None: + logging.basicConfig( + format="%(asctime)-15s %(levelname)-8s %(name)s || %(message)s", + level=level.upper(), ) - logger.addHandler(ch) - # this prevents duplicate logging messages - logger.propagate = False + logging.getLogger("urllib3").setLevel(logging.INFO) # TODO: upstream this into networkx? From 571662928006261d42e6ff5af4b8b0359569e967 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 2 Feb 2024 14:57:16 +0100 Subject: [PATCH 04/24] remove unused argument --- conda_forge_tick/cli.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index ff1146d92..d35de3ead 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -55,7 +55,8 @@ def invoke(self, ctx: click.Context): "--online/--offline", default=False, help="Online: Use the GitHub API for accessing the dependency graph. This is useful for local testing. Note " - "however that any write operations will not be performed.", + "however that any write operations will not be performed. Important: The current working directory will be " + "used to cache JSON files. Local files will be used if they exist.", ) @pass_context def main(ctx: CliContext, debug: bool, dry_run: bool, online: bool) -> None: @@ -76,7 +77,7 @@ def main(ctx: CliContext, debug: bool, dry_run: bool, online: bool) -> None: @main.command(name="gather-all-feedstocks") @pass_context -def gather_all_feedstocks(ctx: CliContext) -> None: +def gather_all_feedstocks() -> None: from . import all_feedstocks all_feedstocks.main() From e1b09d76aa1bd70a0ced21dce3370b7265300a08 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 2 Feb 2024 15:07:01 +0100 Subject: [PATCH 05/24] add tests and error handling --- conda_forge_tick/cli.py | 2 +- conda_forge_tick/lazy_json_backends.py | 2 ++ tests/test_lazy_json_backends.py | 32 ++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index d35de3ead..aa6fd028e 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -54,7 +54,7 @@ def invoke(self, ctx: click.Context): @click.option( "--online/--offline", default=False, - help="Online: Use the GitHub API for accessing the dependency graph. This is useful for local testing. Note " + help="online: Use the GitHub API for accessing the dependency graph. This is useful for local testing. Note " "however that any write operations will not be performed. Important: The current working directory will be " "used to cache JSON files. Local files will be used if they exist.", ) diff --git a/conda_forge_tick/lazy_json_backends.py b/conda_forge_tick/lazy_json_backends.py index c637cf823..dc7bdeaa0 100644 --- a/conda_forge_tick/lazy_json_backends.py +++ b/conda_forge_tick/lazy_json_backends.py @@ -269,6 +269,8 @@ def hget(self, name: str, key: str) -> str: sharded_path = get_sharded_path(f"{name}/{key}.json") url = urllib.parse.urljoin(self.base_url, sharded_path) r = requests.get(url) + if r.status_code == 404: + raise KeyError(f"Key {key} not found in hashmap {name}") r.raise_for_status() return r.text diff --git a/tests/test_lazy_json_backends.py b/tests/test_lazy_json_backends.py index b16499b05..c33ddf9ed 100644 --- a/tests/test_lazy_json_backends.py +++ b/tests/test_lazy_json_backends.py @@ -550,3 +550,35 @@ def test_hexists_failure( mock_head.assert_called_once_with( f"{TestGithubLazyJsonBackend.base_url}/name/key.json", ) + + def test_hkeys(self, backend: GithubLazyJsonBackend) -> None: + assert backend.hkeys("name") == [] + + def test_hgetall(self, backend: GithubLazyJsonBackend) -> None: + assert backend.hgetall("name") == {} + + @mock.patch("requests.get") + def test_hget_success( + self, + mock_get: MagicMock, + backend: GithubLazyJsonBackend, + ) -> None: + mock_get.return_value.status_code = 200 + mock_get.return_value.text = "{'key': 'value'}" + assert backend.hget("name", "key") == "{'key': 'value'}" + mock_get.assert_called_once_with( + f"{TestGithubLazyJsonBackend.base_url}/name/4/4/0/9/d/key.json", + ) + + @mock.patch("requests.get") + def test_hget_not_found( + self, + mock_get: MagicMock, + backend: GithubLazyJsonBackend, + ) -> None: + mock_get.return_value.status_code = 404 + with pytest.raises(KeyError): + backend.hget("name", "key") + mock_get.assert_called_once_with( + f"{TestGithubLazyJsonBackend.base_url}/name/4/4/0/9/d/key.json", + ) From 2edce246dbac962f1f5201cbb9fab5ba0ed3d345 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 2 Feb 2024 15:19:38 +0100 Subject: [PATCH 06/24] Add documentation about local debug functionality. --- README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/README.md b/README.md index e354bea5a..5aa3a789b 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,27 @@ in the `autotick-bot` subdirectory. If you want to stop the worker, rename the file to `please.stop` and it will not restart itself on the next round. + +## Debugging Locally +You can use the CLI of the bot to debug it locally. To do so, install the bot with the following command: +```bash +pip install -e . +``` + +Then you can use the CLI like this: +```bash +conda-forge-tick --help +``` + +For debugging, use the `--debug` flag. This enables debug logging and disables multiprocessing. + +Note that the bot expects the [conda-forge dependency graph](https://github.com/regro/cf-graph-countyfair) to be +present in the current working directory by default. Use `--online` to download missing files on demand from GitHub. + +The local debugging functionality is still work in progress and might not work for all commands. +Currently, the following commands are supported and tested: +- `update-upstream-versions` + ## What has the bot done recently? Check out its [PRs](https://github.com/pulls?utf8=%E2%9C%93&q=is%3Aopen+is%3Apr+author%3Aregro-cf-autotick-bot+archived%3Afalse+), its currently [running jobs](https://github.com/regro/cf-scripts/actions?query=is%3Ain_progress++), and the [status page](https://conda-forge.org/status/#current_migrations)! From 008c8e36581e636dd4e10e29377704b0dfabb4d1 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 2 Feb 2024 16:44:17 +0100 Subject: [PATCH 07/24] raise error when graph file not found --- conda_forge_tick/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda_forge_tick/utils.py b/conda_forge_tick/utils.py index a13df82f7..f98164931 100644 --- a/conda_forge_tick/utils.py +++ b/conda_forge_tick/utils.py @@ -454,7 +454,7 @@ def load_graph(filename: str = "graph.json") -> nx.DiGraph: if dta: return nx.node_link_graph(dta) else: - return None + raise FileNotFoundError("Graph file not found.") # TODO: This type does not support generics yet sadly From c5211d3d7ef91bfddd198ed2dfc3c214f3b99716 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 2 Feb 2024 18:04:44 +0100 Subject: [PATCH 08/24] remove redundant type checking clause --- conda_forge_tick/update_prs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/conda_forge_tick/update_prs.py b/conda_forge_tick/update_prs.py index 9a03d2361..966669baa 100644 --- a/conda_forge_tick/update_prs.py +++ b/conda_forge_tick/update_prs.py @@ -2,7 +2,6 @@ import hashlib import logging import random -import typing from concurrent.futures._base import as_completed import github3 From a32e708a46f10ce71e3dc3d963e16744ce7ef6ff Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 2 Feb 2024 18:08:59 +0100 Subject: [PATCH 09/24] add !TIP about the --online flag --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5aa3a789b..224f89721 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,11 @@ conda-forge-tick --help For debugging, use the `--debug` flag. This enables debug logging and disables multiprocessing. Note that the bot expects the [conda-forge dependency graph](https://github.com/regro/cf-graph-countyfair) to be -present in the current working directory by default. Use `--online` to download missing files on demand from GitHub. +present in the current working directory by default, unless the `--online` flag is used. + +> [!TIP] +> Use the `--online` flag when debugging the bot locally to avoid having to clone the whole +> dependency graph. The local debugging functionality is still work in progress and might not work for all commands. Currently, the following commands are supported and tested: From 52f921ed4086a6c27d5e826f32e40ba420401c83 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 2 Feb 2024 18:46:55 +0100 Subject: [PATCH 10/24] don't group tests into class, test online against GitHub --- tests/test_lazy_json_backends.py | 146 +++++++++++++++++-------------- 1 file changed, 79 insertions(+), 67 deletions(-) diff --git a/tests/test_lazy_json_backends.py b/tests/test_lazy_json_backends.py index c33ddf9ed..60cfa8e1d 100644 --- a/tests/test_lazy_json_backends.py +++ b/tests/test_lazy_json_backends.py @@ -512,73 +512,85 @@ def test_lazy_json_backends_hashmap(tmpdir): assert get_all_keys_for_hashmap("lazy_json") == [] -class TestGithubLazyJsonBackend: - base_url = "https://github.com/lorem/ipsum/raw/master" - - @pytest.fixture - def backend(self) -> GithubLazyJsonBackend: - github_backend = GithubLazyJsonBackend() - github_backend.base_url = self.base_url - return github_backend - - def test_base_url(self) -> None: - github_backend = GithubLazyJsonBackend() - assert github_backend.base_url == GITHUB_GRAPH_BACKEND_BASE_URL + "/" - github_backend.base_url = self.base_url - assert github_backend.base_url == self.base_url + "/" - - @mock.patch("requests.head") - def test_hexists_success( - self, - mock_head: MagicMock, - backend: GithubLazyJsonBackend, - ) -> None: - mock_head.return_value.status_code = 200 - assert backend.hexists("name", "key") - mock_head.assert_called_once_with( - f"{TestGithubLazyJsonBackend.base_url}/name/key.json", - ) +def test_github_base_url() -> None: + github_backend = GithubLazyJsonBackend() + assert github_backend.base_url == GITHUB_GRAPH_BACKEND_BASE_URL + "/" + github_backend.base_url = "https://github.com/lorem/ipsum" + assert github_backend.base_url == "https://github.com/lorem/ipsum" + "/" - @mock.patch("requests.head") - def test_hexists_failure( - self, - mock_head: MagicMock, - backend: GithubLazyJsonBackend, - ) -> None: - mock_head.return_value.status_code = 404 - assert not backend.hexists("name", "key") - mock_head.assert_called_once_with( - f"{TestGithubLazyJsonBackend.base_url}/name/key.json", - ) - def test_hkeys(self, backend: GithubLazyJsonBackend) -> None: - assert backend.hkeys("name") == [] - - def test_hgetall(self, backend: GithubLazyJsonBackend) -> None: - assert backend.hgetall("name") == {} - - @mock.patch("requests.get") - def test_hget_success( - self, - mock_get: MagicMock, - backend: GithubLazyJsonBackend, - ) -> None: - mock_get.return_value.status_code = 200 - mock_get.return_value.text = "{'key': 'value'}" - assert backend.hget("name", "key") == "{'key': 'value'}" - mock_get.assert_called_once_with( - f"{TestGithubLazyJsonBackend.base_url}/name/4/4/0/9/d/key.json", - ) +@pytest.mark.parametrize( + "name, key", + [ + ("node_attrs", "flask"), + ("node_attrs", "requests"), + ("node_attrs", "boto3"), + ("node_attrs", "setuptools"), + ], +) +def test_github_online_hexists_success( + name: str, + key: str, +) -> None: + # this performs a web request + assert GithubLazyJsonBackend().hexists(name, key) - @mock.patch("requests.get") - def test_hget_not_found( - self, - mock_get: MagicMock, - backend: GithubLazyJsonBackend, - ) -> None: - mock_get.return_value.status_code = 404 - with pytest.raises(KeyError): - backend.hget("name", "key") - mock_get.assert_called_once_with( - f"{TestGithubLazyJsonBackend.base_url}/name/4/4/0/9/d/key.json", - ) + +@pytest.mark.parametrize( + "name, key", + [ + ("node_attrs", "this-package-will-not-ever-exist-invalid"), + ("invalid-name", "flask"), + ], +) +def test_github_online_hexists_failure(name: str, key: str) -> None: + # this performs a web request + assert not GithubLazyJsonBackend().hexists(name, key) + + +def test_github_hkeys() -> None: + assert GithubLazyJsonBackend().hkeys("name") == [] + + +def test_github_hgetall() -> None: + assert GithubLazyJsonBackend().hgetall("name") == {} + + +@mock.patch("requests.get") +def test_github_hget_success( + mock_get: MagicMock, +) -> None: + backend = GithubLazyJsonBackend() + backend.base_url = "https://github.com/lorem/ipsum" + mock_get.return_value.status_code = 200 + mock_get.return_value.text = "{'key': 'value'}" + assert backend.hget("name", "key") == "{'key': 'value'}" + mock_get.assert_called_once_with( + f"https://github.com/lorem/ipsum/name/4/4/0/9/d/key.json", + ) + + +@mock.patch("requests.get") +def test_github_offline_hget_not_found( + mock_get: MagicMock, +) -> None: + backend = GithubLazyJsonBackend() + backend.base_url = "https://github.com/lorem/ipsum" + mock_get.return_value.status_code = 404 + with pytest.raises(KeyError): + backend.hget("name", "key") + mock_get.assert_called_once_with( + f"https://github.com/lorem/ipsum/name/4/4/0/9/d/key.json", + ) + + +@pytest.mark.parametrize( + "name, key", + [ + ("node_attrs", "this-package-will-not-ever-exist-invalid"), + ("invalid-name", "flask"), + ], +) +def test_github_online_hget_not_found(name: str, key: str): + with pytest.raises(KeyError): + GithubLazyJsonBackend().hget(name, key) From ef4253f6594cac18947708080ec2f621e8b0fa8a Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 2 Feb 2024 19:21:31 +0100 Subject: [PATCH 11/24] fix: GitHub hexists --- conda_forge_tick/lazy_json_backends.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/conda_forge_tick/lazy_json_backends.py b/conda_forge_tick/lazy_json_backends.py index dc7bdeaa0..2a1bd87a3 100644 --- a/conda_forge_tick/lazy_json_backends.py +++ b/conda_forge_tick/lazy_json_backends.py @@ -232,8 +232,11 @@ def snapshot_context(self) -> "GithubLazyJsonBackend": def hexists(self, name: str, key: str) -> bool: self._inform_web_request() - url = urllib.parse.urljoin(self.base_url, f"{name}/{key}.json") - status = requests.head(url).status_code + url = urllib.parse.urljoin( + self.base_url, + get_sharded_path(f"{name}/{key}.json"), + ) + status = requests.head(url, allow_redirects=True).status_code if status == 200: return True From 0c97b2de849dce32489bcf5536d38d2f52402a0e Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 5 Feb 2024 10:02:33 +0100 Subject: [PATCH 12/24] avoid the term GitHub API for raw URLs --- conda_forge_tick/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index aa6fd028e..a461dd91f 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -54,7 +54,7 @@ def invoke(self, ctx: click.Context): @click.option( "--online/--offline", default=False, - help="online: Use the GitHub API for accessing the dependency graph. This is useful for local testing. Note " + help="online: Make requests to GitHub for accessing the dependency graph. This is useful for local testing. Note " "however that any write operations will not be performed. Important: The current working directory will be " "used to cache JSON files. Local files will be used if they exist.", ) From 6d8d426820fc823c0efded4f0173c5f1eb8280dc Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Thu, 8 Feb 2024 15:34:49 +0100 Subject: [PATCH 13/24] use context manager for overriding backends --- conda_forge_tick/cli.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index a461dd91f..11cdb0023 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -4,7 +4,7 @@ from typing import Optional import click -from click import IntRange +from click import IntRange, Context from conda_forge_tick import lazy_json_backends @@ -59,7 +59,14 @@ def invoke(self, ctx: click.Context): "used to cache JSON files. Local files will be used if they exist.", ) @pass_context -def main(ctx: CliContext, debug: bool, dry_run: bool, online: bool) -> None: +@click.pass_context +def main( + click_context: Context, + ctx: CliContext, + debug: bool, + dry_run: bool, + online: bool, +) -> None: log_level = "debug" if debug else "info" setup_logging(log_level) @@ -71,8 +78,9 @@ def main(ctx: CliContext, debug: bool, dry_run: bool, online: bool) -> None: if online: logger.info("Running in online mode") - lazy_json_backends.CF_TICK_GRAPH_DATA_BACKENDS = ("github",) - lazy_json_backends.CF_TICK_GRAPH_DATA_PRIMARY_BACKEND = "github" + click_context.with_resource( + lazy_json_backends.lazy_json_override_backends(["github"]), + ) @main.command(name="gather-all-feedstocks") From 3cafa90f6079f2461bbe3691a2b5e5430fe7f1f3 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Thu, 8 Feb 2024 18:03:42 +0100 Subject: [PATCH 14/24] correct typing for load_graph handling of empty/nonexistent graph file --- conda_forge_tick/auto_tick.py | 7 ++--- conda_forge_tick/cli.py | 2 +- conda_forge_tick/deploy.py | 4 +-- conda_forge_tick/pypi_name_mapping.py | 4 +-- conda_forge_tick/update_prs.py | 4 +-- conda_forge_tick/update_upstream_versions.py | 4 +-- conda_forge_tick/utils.py | 27 +++++++++++++++++--- 7 files changed, 36 insertions(+), 16 deletions(-) diff --git a/conda_forge_tick/auto_tick.py b/conda_forge_tick/auto_tick.py index de0253b31..22b8cee74 100644 --- a/conda_forge_tick/auto_tick.py +++ b/conda_forge_tick/auto_tick.py @@ -100,6 +100,7 @@ fold_log_lines, frozen_to_json_friendly, get_keys_default, + load_existing_graph, load_graph, parse_meta_yaml, parse_munged_run_export, @@ -982,7 +983,7 @@ def initialize_migrators( dry_run: bool = False, ) -> Tuple[MigratorSessionContext, list, MutableSequence[Migrator]]: temp = glob.glob("/tmp/*") - gx = load_graph() + gx = load_existing_graph() smithy_version = eval_cmd("conda smithy --version").strip() pinning_version = json.loads(eval_cmd("conda list conda-forge-pinning --json"))[0][ "version" @@ -1409,7 +1410,7 @@ def _setup_limits(): resource.setrlimit(resource.RLIMIT_AS, (limit_int, limit_int)) -def _update_nodes_with_bot_rerun(gx): +def _update_nodes_with_bot_rerun(gx: nx.DiGraph): """Go through all the open PRs and check if they are rerun""" print("processing bot-rerun labels", flush=True) @@ -1552,7 +1553,7 @@ def _remove_closed_pr_json(): def _update_graph_with_pr_info(): _remove_closed_pr_json() - gx = load_graph() + gx = load_existing_graph() _update_nodes_with_bot_rerun(gx) _update_nodes_with_new_versions(gx) dump_graph(gx) diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index 11cdb0023..07261ad8e 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -4,7 +4,7 @@ from typing import Optional import click -from click import IntRange, Context +from click import Context, IntRange from conda_forge_tick import lazy_json_backends diff --git a/conda_forge_tick/deploy.py b/conda_forge_tick/deploy.py index 4b5a1a146..047706e60 100644 --- a/conda_forge_tick/deploy.py +++ b/conda_forge_tick/deploy.py @@ -6,7 +6,7 @@ from . import sensitive_env from .cli_context import CliContext from .lazy_json_backends import CF_TICK_GRAPH_DATA_HASHMAPS, get_lazy_json_backends -from .utils import load_graph +from .utils import load_existing_graph BUILD_URL_KEY = "CIRCLE_BUILD_URL" @@ -93,7 +93,7 @@ def deploy(ctx: CliContext): # make sure the graph can load, if not we will error try: - gx = load_graph() + gx = load_existing_graph() # TODO: be more selective about which json to check for node, attrs in gx.nodes.items(): attrs["payload"]._load() diff --git a/conda_forge_tick/pypi_name_mapping.py b/conda_forge_tick/pypi_name_mapping.py index fe4b3e980..46d42de50 100644 --- a/conda_forge_tick/pypi_name_mapping.py +++ b/conda_forge_tick/pypi_name_mapping.py @@ -20,7 +20,7 @@ from packaging.utils import canonicalize_name as canonicalize_pypi_name from .lazy_json_backends import LazyJson, dump, get_all_keys_for_hashmap, loads -from .utils import as_iterable, load_graph +from .utils import as_iterable, load_existing_graph, load_graph class Mapping(TypedDict): @@ -298,7 +298,7 @@ def determine_best_matches_for_pypi_import( map_by_conda_name[conda_name] = m graph_file = str(pathlib.Path(".") / "graph.json") - gx = load_graph(graph_file) + gx = load_existing_graph(graph_file) # TODO: filter out archived feedstocks? try: diff --git a/conda_forge_tick/update_prs.py b/conda_forge_tick/update_prs.py index 966669baa..683c25fec 100644 --- a/conda_forge_tick/update_prs.py +++ b/conda_forge_tick/update_prs.py @@ -18,7 +18,7 @@ from .executors import executor from .make_graph import ghctx -from .utils import github_client, load_graph +from .utils import github_client, load_existing_graph # from conda_forge_tick.profiler import profiling @@ -164,7 +164,7 @@ def close_dirty_prs( # @profiling def main(ctx: CliContext, job: int = 1, n_jobs: int = 1) -> None: - gx = load_graph() + gx = load_existing_graph() gx = close_labels(gx, ctx.dry_run, job=job, n_jobs=n_jobs) gx = update_graph_pr_status(gx, ctx.dry_run, job=job, n_jobs=n_jobs) diff --git a/conda_forge_tick/update_upstream_versions.py b/conda_forge_tick/update_upstream_versions.py index 53abf58a5..1d91bac9b 100644 --- a/conda_forge_tick/update_upstream_versions.py +++ b/conda_forge_tick/update_upstream_versions.py @@ -36,7 +36,7 @@ RawURL, ROSDistro, ) -from .utils import get_keys_default, load_graph +from .utils import get_keys_default, load_existing_graph T = TypeVar("T") @@ -405,7 +405,7 @@ def main( """ logger.info("Reading graph") # Graph enabled for inspection - gx = load_graph() + gx = load_existing_graph() # Check if 'versions' folder exists or create a new one; os.makedirs("versions", exist_ok=True) diff --git a/conda_forge_tick/utils.py b/conda_forge_tick/utils.py index f98164931..1822566f2 100644 --- a/conda_forge_tick/utils.py +++ b/conda_forge_tick/utils.py @@ -74,6 +74,8 @@ def _munge_dict_repr(d): datetime=datetime, ) +DEFAULT_GRAPH_FILENAME = "graph.json" + @contextlib.contextmanager def fold_log_lines(title): @@ -431,7 +433,6 @@ def dump_graph_json(gx: nx.DiGraph, filename: str = "graph.json") -> None: links = nld["links"] links2 = sorted(links, key=lambda x: f'{x["source"]}{x["target"]}') nld["links"] = links2 - from conda_forge_tick.lazy_json_backends import LazyJson lzj = LazyJson(filename) with lzj as attrs: @@ -447,14 +448,32 @@ def dump_graph( dump_graph_json(gx, filename) -def load_graph(filename: str = "graph.json") -> nx.DiGraph: - from conda_forge_tick.lazy_json_backends import LazyJson +def load_existing_graph(filename: str = DEFAULT_GRAPH_FILENAME) -> nx.DiGraph: + """ + Load the graph from a file using the lazy json backend. + If a non-existing or empty file is encountered, a FileNotFoundError is raised. + If you expect the graph to possibly not exist, use load_graph. + + :return: the graph + """ + gx = load_graph(filename) + if gx is None: + raise FileNotFoundError(f"Graph file {filename} does not exist or is empty") + return gx + +def load_graph(filename: str = DEFAULT_GRAPH_FILENAME) -> Optional[nx.DiGraph]: + """ + Load the graph from a file using the lazy json backend. + If you expect the graph to exist, use load_existing_graph. + + :return: the graph, or None if the file does not exist or is empty JSON + """ dta = copy.deepcopy(LazyJson(filename).data) if dta: return nx.node_link_graph(dta) else: - raise FileNotFoundError("Graph file not found.") + return None # TODO: This type does not support generics yet sadly From b9152fcf6b744060d63c02a56d4522c4a98e132c Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Fri, 9 Feb 2024 09:34:51 +0100 Subject: [PATCH 15/24] add missing import --- conda_forge_tick/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index 07261ad8e..05bb8a3e4 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -7,6 +7,7 @@ from click import Context, IntRange from conda_forge_tick import lazy_json_backends +from conda_forge_tick.utils import setup_logging from .cli_context import CliContext From 3ddcf88ae866590e62aa82a3411df1a169f0ad5a Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 12 Feb 2024 11:50:04 +0100 Subject: [PATCH 16/24] fix test: load_graph --- tests/test_upstream_versions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_upstream_versions.py b/tests/test_upstream_versions.py index 01e9585e8..148f76373 100644 --- a/tests/test_upstream_versions.py +++ b/tests/test_upstream_versions.py @@ -1590,7 +1590,7 @@ def test_update_upstream_versions_process_pool_exception_repr_exception( @pytest.mark.parametrize("debug", [True, False]) @mock.patch("os.makedirs") -@mock.patch("conda_forge_tick.update_upstream_versions.load_graph") +@mock.patch("conda_forge_tick.update_upstream_versions.load_existing_graph") @mock.patch("conda_forge_tick.update_upstream_versions.update_upstream_versions") def test_main( update_upstream_versions_mock: MagicMock, From 2f8b64c6469a7a77580a85a86f44896d6a39115e Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 12 Feb 2024 12:48:08 +0100 Subject: [PATCH 17/24] gather_all_feedstocks does not take context --- conda_forge_tick/cli.py | 1 - tests/test_cli.py | 1 - 2 files changed, 2 deletions(-) diff --git a/conda_forge_tick/cli.py b/conda_forge_tick/cli.py index 05bb8a3e4..59510e2a5 100644 --- a/conda_forge_tick/cli.py +++ b/conda_forge_tick/cli.py @@ -85,7 +85,6 @@ def main( @main.command(name="gather-all-feedstocks") -@pass_context def gather_all_feedstocks() -> None: from . import all_feedstocks diff --git a/tests/test_cli.py b/tests/test_cli.py index 0ce20f7e0..da81c7bb8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -71,7 +71,6 @@ def test_invalid_job(command: str, job: int, n_jobs: int): takes_context_commands = ( - "gather-all-feedstocks", "make-graph", "update-upstream-versions", "auto-tick", From f569f6350f24ea865062cb0b63a0777d76f354a3 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 12 Feb 2024 13:29:15 +0100 Subject: [PATCH 18/24] hkeys and hgetall is not implemented in GitHubLazyJsonBackend --- conda_forge_tick/lazy_json_backends.py | 17 +++++++++-------- tests/test_lazy_json_backends.py | 6 ++++-- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/conda_forge_tick/lazy_json_backends.py b/conda_forge_tick/lazy_json_backends.py index 2a1bd87a3..c8746a7d8 100644 --- a/conda_forge_tick/lazy_json_backends.py +++ b/conda_forge_tick/lazy_json_backends.py @@ -260,12 +260,12 @@ def hdel(self, name: str, keys: Iterable[str]) -> None: def hkeys(self, name: str) -> List[str]: """ Not implemented for GithubLazyJsonBackend. - Logs a warning and returns an empty list. + Raises an error. """ - logger.warning( - "hkeys not implemented for GithubLazyJsonBackend. Returning empty list.", + raise NotImplementedError( + "hkeys not implemented for GitHub JSON backend." + "You cannot use the GitHub backend with operations that require listing all hashmap keys." ) - return [] def hget(self, name: str, key: str) -> str: self._inform_web_request() @@ -280,12 +280,13 @@ def hget(self, name: str, key: str) -> str: def hgetall(self, name: str, hashval: bool = False) -> Dict[str, str]: """ Not implemented for GithubLazyJsonBackend. - Logs a warning and returns an empty dict. + Raises an error. """ - logger.warning( - "hgetall not implemented for GithubLazyJsonBackend. Returning empty dict.", + raise NotImplementedError( + "hgetall not implemented for GitHub JSON backend." + "You cannot use the GitHub backend as source or target for hashmap synchronization or other" + "commands that require listing all hashmap keys.", ) - return {} @functools.lru_cache(maxsize=128) diff --git a/tests/test_lazy_json_backends.py b/tests/test_lazy_json_backends.py index 60cfa8e1d..3799ec115 100644 --- a/tests/test_lazy_json_backends.py +++ b/tests/test_lazy_json_backends.py @@ -549,11 +549,13 @@ def test_github_online_hexists_failure(name: str, key: str) -> None: def test_github_hkeys() -> None: - assert GithubLazyJsonBackend().hkeys("name") == [] + with pytest.raises(NotImplementedError): + assert GithubLazyJsonBackend().hkeys("name") == [] def test_github_hgetall() -> None: - assert GithubLazyJsonBackend().hgetall("name") == {} + with pytest.raises(NotImplementedError): + GithubLazyJsonBackend().hgetall("name") @mock.patch("requests.get") From 982bb883a25ada2a0d15fada397de39ff6e42f09 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 12 Feb 2024 14:19:21 +0100 Subject: [PATCH 19/24] add tests for GitHub backend --- conda_forge_tick/status_report.py | 1 - tests/test_lazy_json_backends.py | 75 +++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/conda_forge_tick/status_report.py b/conda_forge_tick/status_report.py index 2e0f4bd47..1d75565c3 100644 --- a/conda_forge_tick/status_report.py +++ b/conda_forge_tick/status_report.py @@ -17,7 +17,6 @@ from graphviz import Source from conda_forge_tick.auto_tick import _filter_ignored_versions, initialize_migrators -from conda_forge_tick.cli_context import CliContext from conda_forge_tick.contexts import FeedstockContext from conda_forge_tick.lazy_json_backends import LazyJson, get_all_keys_for_hashmap from conda_forge_tick.migrators import ( diff --git a/tests/test_lazy_json_backends.py b/tests/test_lazy_json_backends.py index 3799ec115..230e01524 100644 --- a/tests/test_lazy_json_backends.py +++ b/tests/test_lazy_json_backends.py @@ -1,5 +1,6 @@ import hashlib import json +import logging import os import pickle from unittest import mock @@ -548,6 +549,80 @@ def test_github_online_hexists_failure(name: str, key: str) -> None: assert not GithubLazyJsonBackend().hexists(name, key) +@mock.patch("requests.head") +def test_github_hexists_unexpected_status_code(request_mock: MagicMock) -> None: + request_mock.return_value.status_code = 500 + + with pytest.raises(RuntimeError, match="Unexpected status code 500"): + GithubLazyJsonBackend().hexists("name", "key") + + +def test_github_hdel(caplog) -> None: + caplog.set_level(logging.DEBUG) + backend = GithubLazyJsonBackend() + backend.hdel("name", ["key1", "key2"]) + + assert "Write operations to the GitHub online backend are ignored." in caplog.text + + backend.hdel("name2", ["key3"]) + + # warning should only be once in log + assert ( + caplog.text.count("Write operations to the GitHub online backend are ignored") + == 1 + ) + + +def test_github_hmset(caplog) -> None: + caplog.set_level(logging.DEBUG) + backend = GithubLazyJsonBackend() + backend.hmset("name", {"a": "b"}) + + assert "Write operations to the GitHub online backend are ignored." in caplog.text + + backend.hmset("name2", {}) + + # warning should only be once in log + assert ( + caplog.text.count("Write operations to the GitHub online backend are ignored") + == 1 + ) + + +def test_github_hset(caplog) -> None: + caplog.set_level(logging.DEBUG) + backend = GithubLazyJsonBackend() + backend.hset("name", "key", "value") + + assert "Write operations to the GitHub online backend are ignored." in caplog.text + + backend.hset("name", "key", "value2") + + # warning should only be once in log + assert ( + caplog.text.count("Write operations to the GitHub online backend are ignored") + == 1 + ) + + +def test_github_write_mix(caplog) -> None: + caplog.set_level(logging.DEBUG) + backend = GithubLazyJsonBackend() + + backend.hset("name", "key", "value") + backend.hdel("name", ["a", "b"]) + backend.hmset("name2", {"a": "d"}) + backend.hset("name", "key", "value") + backend.hdel("name3", ["a", "b"]) + backend.hmset("name", {"a": "d"}) + + # warning should only be once in log + assert ( + caplog.text.count("Write operations to the GitHub online backend are ignored") + == 1 + ) + + def test_github_hkeys() -> None: with pytest.raises(NotImplementedError): assert GithubLazyJsonBackend().hkeys("name") == [] From d3107036bb75a6382c12c73b645531f64d03aeff Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 12 Feb 2024 15:20:43 +0100 Subject: [PATCH 20/24] add tests for load_graph, document behaviour --- conda_forge_tick/auto_tick.py | 1 - conda_forge_tick/pypi_name_mapping.py | 2 +- conda_forge_tick/utils.py | 14 ++- tests/test_utils.py | 129 ++++++++++++++++++++++++-- 4 files changed, 132 insertions(+), 14 deletions(-) diff --git a/conda_forge_tick/auto_tick.py b/conda_forge_tick/auto_tick.py index 22b8cee74..519c0aef8 100644 --- a/conda_forge_tick/auto_tick.py +++ b/conda_forge_tick/auto_tick.py @@ -101,7 +101,6 @@ frozen_to_json_friendly, get_keys_default, load_existing_graph, - load_graph, parse_meta_yaml, parse_munged_run_export, pluck, diff --git a/conda_forge_tick/pypi_name_mapping.py b/conda_forge_tick/pypi_name_mapping.py index 46d42de50..7a338c11c 100644 --- a/conda_forge_tick/pypi_name_mapping.py +++ b/conda_forge_tick/pypi_name_mapping.py @@ -20,7 +20,7 @@ from packaging.utils import canonicalize_name as canonicalize_pypi_name from .lazy_json_backends import LazyJson, dump, get_all_keys_for_hashmap, loads -from .utils import as_iterable, load_existing_graph, load_graph +from .utils import as_iterable, load_existing_graph class Mapping(TypedDict): diff --git a/conda_forge_tick/utils.py b/conda_forge_tick/utils.py index 1822566f2..7f524659a 100644 --- a/conda_forge_tick/utils.py +++ b/conda_forge_tick/utils.py @@ -451,23 +451,27 @@ def dump_graph( def load_existing_graph(filename: str = DEFAULT_GRAPH_FILENAME) -> nx.DiGraph: """ Load the graph from a file using the lazy json backend. - If a non-existing or empty file is encountered, a FileNotFoundError is raised. - If you expect the graph to possibly not exist, use load_graph. + If the file does not exist, it is initialized with empty JSON before performing any reads. + If empty JSON is encountered, a ValueError is raised. + If you expect the graph to be possibly empty JSON (i.e. not initialized), use load_graph. :return: the graph + :raises ValueError if the file contains empty JSON (or did not exist before) """ gx = load_graph(filename) if gx is None: - raise FileNotFoundError(f"Graph file {filename} does not exist or is empty") + raise ValueError(f"Graph file {filename} contains empty JSON") return gx def load_graph(filename: str = DEFAULT_GRAPH_FILENAME) -> Optional[nx.DiGraph]: """ Load the graph from a file using the lazy json backend. - If you expect the graph to exist, use load_existing_graph. + If the file does not exist, it is initialized with empty JSON. + If you expect the graph to be non-empty JSON, use load_existing_graph. - :return: the graph, or None if the file does not exist or is empty JSON + :return: the graph, or None if the file is empty JSON (or + :raises FileNotFoundError if the file does not exist """ dta = copy.deepcopy(LazyJson(filename).data) if dta: diff --git a/tests/test_utils.py b/tests/test_utils.py index 401ef05b5..58323746c 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,4 +1,52 @@ -from conda_forge_tick.utils import get_keys_default +from unittest import mock +from unittest.mock import MagicMock, mock_open + +import pytest + +from conda_forge_tick.utils import ( + DEFAULT_GRAPH_FILENAME, + get_keys_default, + load_existing_graph, + load_graph, +) + +EMPTY_JSON = "{}" +DEMO_GRAPH = """ +{ + "directed": true, + "graph": { + "outputs_lut": { + "package1": { + "__set__": true, + "elements": [ + "package1" + ] + }, + "package2": { + "__set__": true, + "elements": [ + "package2" + ] + } + } + }, + "links": [ + { + "source": "package1", + "target": "package2" + } + ], + "multigraph": false, + "nodes": [ + { + "id": "package1", + "payload": { + "__lazy_json__": "node_attrs/package1.json" + } + } + ] +} +""" def test_get_keys_default(): @@ -25,9 +73,76 @@ def test_get_keys_default_none(): "bot": None, }, } - get_keys_default( - attrs, - ["conda-forge.yml", "bot", "check_solvable"], - {}, - False, - ) is False + assert ( + get_keys_default( + attrs, + ["conda-forge.yml", "bot", "check_solvable"], + {}, + False, + ) + is False + ) + + +def test_load_graph(): + with mock.patch("builtins.open", mock_open(read_data=DEMO_GRAPH)) as mock_file: + gx = load_graph() + + assert gx is not None + + assert gx.nodes.keys() == {"package1", "package2"} + + mock_file: MagicMock + mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)]) + + +def test_load_graph_empty_graph(): + with mock.patch("builtins.open", mock_open(read_data=EMPTY_JSON)) as mock_file: + gx = load_graph() + + assert gx is None + + mock_file: MagicMock + mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)]) + + +@mock.patch("os.path.exists") +def test_load_graph_file_does_not_exist(exists_mock: MagicMock): + exists_mock.return_value = False + + with mock.patch("builtins.open", mock_open(read_data=EMPTY_JSON)) as mock_file: + load_graph() + + mock_file: MagicMock + mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME, "w")]) + + +def test_load_existing_graph(): + with mock.patch("builtins.open", mock_open(read_data=DEMO_GRAPH)) as mock_file: + gx = load_existing_graph() + + assert gx.nodes.keys() == {"package1", "package2"} + + mock_file: MagicMock + mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)]) + + +def test_load_existing_graph_empty_graph(): + with mock.patch("builtins.open", mock_open(read_data=EMPTY_JSON)) as mock_file: + with pytest.raises(ValueError, match="empty JSON"): + load_existing_graph() + + mock_file: MagicMock + mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)]) + + +@mock.patch("os.path.exists") +def test_load_existing_graph_file_does_not_exist(exists_mock: MagicMock): + exists_mock.return_value = False + + with mock.patch("builtins.open", mock_open(read_data=EMPTY_JSON)) as mock_file: + with pytest.raises(ValueError, match="empty JSON"): + load_existing_graph() + + mock_file: MagicMock + mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME, "w")]) From 6e7205b70e6004da2da40b36d599a22fc1f059cb Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 12 Feb 2024 15:37:04 +0100 Subject: [PATCH 21/24] try moving caplog set level down --- tests/test_lazy_json_backends.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_lazy_json_backends.py b/tests/test_lazy_json_backends.py index 230e01524..ad9f94e94 100644 --- a/tests/test_lazy_json_backends.py +++ b/tests/test_lazy_json_backends.py @@ -590,8 +590,8 @@ def test_github_hmset(caplog) -> None: def test_github_hset(caplog) -> None: - caplog.set_level(logging.DEBUG) backend = GithubLazyJsonBackend() + caplog.set_level(logging.DEBUG) backend.hset("name", "key", "value") assert "Write operations to the GitHub online backend are ignored." in caplog.text From a6a5b8bc564ee46a36a950136b046abf30f8ad15 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 12 Feb 2024 15:42:35 +0100 Subject: [PATCH 22/24] Revert "try moving caplog set level down" This reverts commit 02101bcfa3066f17c57e240c61048031cb1bdf37. --- tests/test_lazy_json_backends.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_lazy_json_backends.py b/tests/test_lazy_json_backends.py index ad9f94e94..230e01524 100644 --- a/tests/test_lazy_json_backends.py +++ b/tests/test_lazy_json_backends.py @@ -590,8 +590,8 @@ def test_github_hmset(caplog) -> None: def test_github_hset(caplog) -> None: - backend = GithubLazyJsonBackend() caplog.set_level(logging.DEBUG) + backend = GithubLazyJsonBackend() backend.hset("name", "key", "value") assert "Write operations to the GitHub online backend are ignored." in caplog.text From 2b6f1bdc64e20d634fc2be03bc3b1580504cc710 Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Mon, 12 Feb 2024 16:14:13 +0100 Subject: [PATCH 23/24] reset GitHub backend in-between tests --- tests/test_lazy_json_backends.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/test_lazy_json_backends.py b/tests/test_lazy_json_backends.py index 230e01524..03dd92e4e 100644 --- a/tests/test_lazy_json_backends.py +++ b/tests/test_lazy_json_backends.py @@ -557,7 +557,16 @@ def test_github_hexists_unexpected_status_code(request_mock: MagicMock) -> None: GithubLazyJsonBackend().hexists("name", "key") -def test_github_hdel(caplog) -> None: +@pytest.fixture +def reset_github_backend(): + # In the future, the program architecture should be changed such that backend instances are shared, instead of + # instantiating each backend multiple times (whenever it is needed). Then, this can be moved into instance + # variables that don't need to be reset. + GithubLazyJsonBackend._write_warned = False + GithubLazyJsonBackend._n_requests = 0 + + +def test_github_hdel(caplog, reset_github_backend) -> None: caplog.set_level(logging.DEBUG) backend = GithubLazyJsonBackend() backend.hdel("name", ["key1", "key2"]) @@ -573,7 +582,7 @@ def test_github_hdel(caplog) -> None: ) -def test_github_hmset(caplog) -> None: +def test_github_hmset(caplog, reset_github_backend) -> None: caplog.set_level(logging.DEBUG) backend = GithubLazyJsonBackend() backend.hmset("name", {"a": "b"}) @@ -589,7 +598,7 @@ def test_github_hmset(caplog) -> None: ) -def test_github_hset(caplog) -> None: +def test_github_hset(caplog, reset_github_backend) -> None: caplog.set_level(logging.DEBUG) backend = GithubLazyJsonBackend() backend.hset("name", "key", "value") @@ -605,7 +614,7 @@ def test_github_hset(caplog) -> None: ) -def test_github_write_mix(caplog) -> None: +def test_github_write_mix(caplog, reset_github_backend) -> None: caplog.set_level(logging.DEBUG) backend = GithubLazyJsonBackend() From 774aff04eed877955a34de6346d474cff98ab51a Mon Sep 17 00:00:00 2001 From: Yannik Tausch Date: Tue, 13 Feb 2024 18:24:17 +0100 Subject: [PATCH 24/24] fix types --- conda_forge_tick/lazy_json_backends.py | 18 +++++++++--------- tests/test_utils.py | 6 ------ 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/conda_forge_tick/lazy_json_backends.py b/conda_forge_tick/lazy_json_backends.py index c8746a7d8..f3fecfd2b 100644 --- a/conda_forge_tick/lazy_json_backends.py +++ b/conda_forge_tick/lazy_json_backends.py @@ -58,12 +58,12 @@ def get_sharded_path(file_path, n_dirs=5): class LazyJsonBackend(ABC): @contextlib.contextmanager @abstractmethod - def transaction_context(self) -> "LazyJsonBackend": + def transaction_context(self) -> "Iterator[LazyJsonBackend]": pass @contextlib.contextmanager @abstractmethod - def snapshot_context(self) -> "LazyJsonBackend": + def snapshot_context(self) -> "Iterator[LazyJsonBackend]": pass @abstractmethod @@ -108,12 +108,12 @@ def hgetall(self, name: str, hashval: bool = False) -> Dict[str, str]: class FileLazyJsonBackend(LazyJsonBackend): @contextlib.contextmanager - def transaction_context(self) -> "FileLazyJsonBackend": + def transaction_context(self) -> "Iterator[FileLazyJsonBackend]": # context not required yield self @contextlib.contextmanager - def snapshot_context(self) -> "FileLazyJsonBackend": + def snapshot_context(self) -> "Iterator[FileLazyJsonBackend]": # context not required yield self @@ -131,7 +131,7 @@ def hmset(self, name: str, mapping: Mapping[str, str]) -> None: for key, value in mapping.items(): self.hset(name, key, value) - def hmget(self, name: str, keys: str) -> List[str]: + def hmget(self, name: str, keys: Iterable[str]) -> List[str]: return [self.hget(name, key) for key in keys] def hgetall(self, name: str, hashval: bool = False) -> Dict[str, str]: @@ -222,11 +222,11 @@ def _inform_web_request(cls): f"Using the GitHub online backend for making a lot of requests is not recommended.", ) - def transaction_context(self) -> "GithubLazyJsonBackend": + def transaction_context(self) -> "Iterator[GithubLazyJsonBackend]": # context not required yield self - def snapshot_context(self) -> "GithubLazyJsonBackend": + def snapshot_context(self) -> "Iterator[GithubLazyJsonBackend]": # context not required yield self @@ -318,7 +318,7 @@ class MongoDBLazyJsonBackend(LazyJsonBackend): _snapshot_session = None @contextlib.contextmanager - def transaction_context(self): + def transaction_context(self) -> "Iterator[MongoDBLazyJsonBackend]": try: if self.__class__._session is None: client = get_graph_data_mongodb_client() @@ -333,7 +333,7 @@ def transaction_context(self): self.__class__._session = None @contextlib.contextmanager - def snapshot_context(self): + def snapshot_context(self) -> "Iterator[MongoDBLazyJsonBackend]": try: if self.__class__._snapshot_session is None: client = get_graph_data_mongodb_client() diff --git a/tests/test_utils.py b/tests/test_utils.py index 58323746c..4ae7c6552 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -92,7 +92,6 @@ def test_load_graph(): assert gx.nodes.keys() == {"package1", "package2"} - mock_file: MagicMock mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)]) @@ -102,7 +101,6 @@ def test_load_graph_empty_graph(): assert gx is None - mock_file: MagicMock mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)]) @@ -113,7 +111,6 @@ def test_load_graph_file_does_not_exist(exists_mock: MagicMock): with mock.patch("builtins.open", mock_open(read_data=EMPTY_JSON)) as mock_file: load_graph() - mock_file: MagicMock mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME, "w")]) @@ -123,7 +120,6 @@ def test_load_existing_graph(): assert gx.nodes.keys() == {"package1", "package2"} - mock_file: MagicMock mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)]) @@ -132,7 +128,6 @@ def test_load_existing_graph_empty_graph(): with pytest.raises(ValueError, match="empty JSON"): load_existing_graph() - mock_file: MagicMock mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME)]) @@ -144,5 +139,4 @@ def test_load_existing_graph_file_does_not_exist(exists_mock: MagicMock): with pytest.raises(ValueError, match="empty JSON"): load_existing_graph() - mock_file: MagicMock mock_file.assert_has_calls([mock.call(DEFAULT_GRAPH_FILENAME, "w")])