diff --git a/README.md b/README.md index d601221..9e3b1f1 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,22 @@ This friction complicates the responsible acceptance of pull requests that chang ## Example -image +```markdown +### Detected 6 changes to dependencies in Poetry lockfile + +From base f4e6ca0f4d67d9bb3f8ab43a89ceca2d0d2be7a1 to target a86b84f85d0bb2bf2fca6d6e8c58f2ce6f9e393c: + +Added **pydantic** (1.10.6) +Added **requests-mock** (1.10.0) +Added **six** (1.16.0) +Added **tomli** (2.0.1) +Added **typing-extensions** (4.5.0) +Updated **urllib3** (1.26.14 -> 1.26.15) + +*(5 added, 0 removed, 1 updated, 4 not changed)* + +Generated by diff-poetry-lock 1.0.1 +``` ## Usage diff --git a/diff_poetry_lock/__init__.py b/diff_poetry_lock/__init__.py index e69de29..f89d482 100644 --- a/diff_poetry_lock/__init__.py +++ b/diff_poetry_lock/__init__.py @@ -0,0 +1,6 @@ +from importlib.metadata import PackageNotFoundError, version + +try: + __version__ = version("diff-poetry-lock") +except PackageNotFoundError: + __version__ = "dev" diff --git a/diff_poetry_lock/github.py b/diff_poetry_lock/github.py index 5b05f72..fb60628 100644 --- a/diff_poetry_lock/github.py +++ b/diff_poetry_lock/github.py @@ -1,7 +1,8 @@ +import base64 + import requests from loguru import logger from pydantic import BaseModel, Field, parse_obj_as -from requests import Response from diff_poetry_lock.settings import PrLookupConfigurable, Settings @@ -30,6 +31,7 @@ class GithubApi: def __init__(self, settings: Settings) -> None: self.s = settings self.session = requests.session() + self._ref_hash_cache: dict[str, str] = {} if isinstance(self.s, PrLookupConfigurable): self.s.set_pr_lookup_service(self) @@ -45,7 +47,7 @@ def post_comment(self, comment: str) -> None: logger.debug("Posting comment to PR #{}", self.s.pr_num) r = self.session.post( f"{self.s.api_url}/repos/{self.s.repository}/issues/{self.s.pr_num}/comments", - headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"}, + headers=self.api_headers(), json={"body": f"{MAGIC_COMMENT_IDENTIFIER}{comment}"}, timeout=10, ) @@ -56,7 +58,7 @@ def update_comment(self, comment_id: int, comment: str) -> None: logger.debug("Updating comment {}", comment_id) r = self.session.patch( f"{self.s.api_url}/repos/{self.s.repository}/issues/comments/{comment_id}", - headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"}, + headers=self.api_headers(), json={"body": f"{MAGIC_COMMENT_IDENTIFIER}{comment}"}, timeout=10, ) @@ -74,7 +76,7 @@ def list_comments(self) -> list[GithubComment]: r = self.session.get( f"{self.s.api_url}/repos/{self.s.repository}/issues/{self.s.pr_num}/comments", params={"per_page": 100, "page": page}, - headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"}, + headers=self.api_headers(), timeout=10, ) r.raise_for_status() @@ -84,28 +86,47 @@ def list_comments(self) -> list[GithubComment]: logger.debug("Found %d comments", len(all_comments)) return [c for c in all_comments if c.is_diff_comment()] - def get_file(self, ref: str) -> Response: + def get_file(self, ref: str) -> bytes: logger.debug("Fetching {} from ref {}", self.s.lockfile_path, ref) r = self.session.get( f"{self.s.api_url}/repos/{self.s.repository}/contents/{self.s.lockfile_path}", params={"ref": ref}, - headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github.raw"}, + headers=self.api_headers(), timeout=10, - stream=True, ) logger.debug("Response status: {}", r.status_code) if r.status_code == 404: raise FileNotFoundError(self.s.lockfile_path) from RepoFileRetrievalError(self.s.repository, ref) r.raise_for_status() - return r + file_obj = r.json() + + resolved_hash = str(file_obj.get("sha", "")).strip() + if resolved_hash: + self._ref_hash_cache[ref] = resolved_hash + logger.debug("Cached commit hash for ref {} from contents sha", ref) + + encoded_content = file_obj.get("content", "") + if not isinstance(encoded_content, str): + msg = "Invalid content returned from GitHub contents API" + raise TypeError(msg) + + return base64.b64decode(encoded_content) + + def resolve_commit_hash(self, ref: str) -> str: + if cached_hash := self._ref_hash_cache.get(ref): + logger.debug("Using cached commit hash for ref {}", ref) + return cached_hash + + logger.warning("No cached commit hash for ref {}, falling back to ref", ref) + return ref def delete_comment(self, comment_id: int) -> None: logger.debug("Deleting comment {}", comment_id) r = self.session.delete( f"{self.s.api_url}/repos/{self.s.repository}/issues/comments/{comment_id}", - headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"}, + headers=self.api_headers(), ) logger.debug("Response status: {}", r.status_code) r.raise_for_status() @@ -122,7 +143,7 @@ def find_pr_for_branch(self, branch_ref: str) -> str: r = self.session.get( f"{self.s.api_url}/repos/{self.s.repository}/pulls", params={"head": head, "state": "open"}, - headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"}, + headers=self.api_headers(), timeout=10, ) logger.debug("Response status: {}", r.status_code) @@ -137,6 +158,13 @@ def find_pr_for_branch(self, branch_ref: str) -> str: logger.debug("No open PR found for branch {}", branch) return "" + def api_headers(self) -> dict[str, str]: + return self.request_headers(self.s.token) + + @staticmethod + def request_headers(token: str) -> dict[str, str]: + return {"Authorization": f"Bearer {token}", "Accept": "application/vnd.github+json"} + def upsert_comment(self, existing_comment: GithubComment | None, comment: str | None) -> None: if existing_comment is None and comment is None: return diff --git a/diff_poetry_lock/run_poetry.py b/diff_poetry_lock/run_poetry.py index 807dca8..18dbf17 100644 --- a/diff_poetry_lock/run_poetry.py +++ b/diff_poetry_lock/run_poetry.py @@ -7,6 +7,7 @@ from poetry.core.packages.package import Package from poetry.packages import Locker +from diff_poetry_lock import __version__ from diff_poetry_lock.github import GithubApi from diff_poetry_lock.logging_utils import configure_logging from diff_poetry_lock.settings import Settings, determine_and_load_settings @@ -79,7 +80,11 @@ def post_comment(api: GithubApi, comment: str | None) -> None: api.upsert_comment(existing_comment, comment) -def format_comment(packages: list[PackageSummary]) -> str | None: +def format_comment( + packages: list[PackageSummary], + base_commit_hash: str | None = None, + target_commit_hash: str | None = None, +) -> str | None: added = sorted([p for p in packages if p.added()], key=attrgetter("name")) removed = sorted([p for p in packages if p.removed()], key=attrgetter("name")) updated = sorted([p for p in packages if p.updated()], key=attrgetter("name")) @@ -88,20 +93,25 @@ def format_comment(packages: list[PackageSummary]) -> str | None: if len(added + removed + updated) == 0: return None - comment = f"### Detected {len(added + removed + updated)} changes to dependencies in Poetry lockfile\n\n" - comment += "\n".join(p.summary_line() for p in added + removed + updated) + change_count = len(added + removed + updated) + comment = f"### Detected {change_count} changes to dependencies in Poetry lockfile\n\n" + if base_commit_hash and target_commit_hash: + comment += f"From base {base_commit_hash} to target {target_commit_hash}:\n\n" + summary_lines = [p.summary_line() for p in added + removed + updated] + comment += "\n".join(summary_lines) comment += ( f"\n\n*({len(added)} added, {len(removed)} removed, {len(updated)} updated, {len(not_changed)} not changed)*" ) + if __version__: + comment += f"\n\nGenerated by diff-poetry-lock {__version__}\n\n" return comment def load_lockfile(api: GithubApi, ref: str) -> list[Package]: - r = api.get_file(ref) + file_contents = api.get_file(ref) with tempfile.NamedTemporaryFile(mode="wb", delete=True) as f: - for chunk in r.iter_content(chunk_size=1024): - f.write(chunk) + f.write(file_contents) f.flush() return load_packages(Path(f.name)) @@ -128,7 +138,17 @@ def do_diff(settings: Settings) -> None: logger.debug("Computing diff...") packages = diff(base_packages, head_packages) - summary = format_comment(packages) + + if not any(package.changed() for package in packages): + summary = None + else: + base_commit_hash = api.resolve_commit_hash(settings.ref) + target_commit_hash = api.resolve_commit_hash(settings.base_ref) + summary = format_comment( + packages, + base_commit_hash=base_commit_hash, + target_commit_hash=target_commit_hash, + ) if summary: logger.debug("Generated summary with {} characters", len(summary)) diff --git a/diff_poetry_lock/test/test_poetry_diff.py b/diff_poetry_lock/test/test_poetry_diff.py index 3d7afad..6a6887b 100644 --- a/diff_poetry_lock/test/test_poetry_diff.py +++ b/diff_poetry_lock/test/test_poetry_diff.py @@ -1,14 +1,17 @@ +import base64 from operator import attrgetter from pathlib import Path from textwrap import dedent -from typing import Any +from typing import Any, Never import pytest +import requests import requests_mock from _pytest.monkeypatch import MonkeyPatch from requests_mock import Mocker -from diff_poetry_lock.github import MAGIC_COMMENT_IDENTIFIER +from diff_poetry_lock import __version__ +from diff_poetry_lock.github import MAGIC_COMMENT_IDENTIFIER, GithubApi from diff_poetry_lock.run_poetry import PackageSummary, diff, do_diff, format_comment, load_packages, main from diff_poetry_lock.settings import GitHubActionsSettings, Settings @@ -74,15 +77,33 @@ def test_diff() -> None: ] assert summary == expected - expected_comment = """\ + expected_comment = f"""\ ### Detected 3 changes to dependencies in Poetry lockfile + From base new-sha to target old-sha: + Removed **pydantic** (1.10.6) Removed **typing-extensions** (4.5.0) Updated **urllib3** (1.26.15 -> 1.26.14) - *(0 added, 2 removed, 1 updated, 4 not changed)*""" - assert format_comment(summary) == dedent(expected_comment) + *(0 added, 2 removed, 1 updated, 4 not changed)* + + Generated by diff-poetry-lock {__version__} + """ + assert ( + format_comment( + summary, + base_commit_hash="new-sha", + target_commit_hash="old-sha", + ) + or "" + ).strip() == dedent(expected_comment).strip() + + +def test_request_headers_method() -> None: + headers = GithubApi.request_headers("sekret-token") + assert headers["Authorization"] == "Bearer sekret-token" + assert headers["Accept"] == "application/vnd.github+json" def test_diff_2() -> None: @@ -102,15 +123,27 @@ def test_diff_2() -> None: ] assert summary == expected - expected_comment = """\ + expected_comment = f"""\ ### Detected 3 changes to dependencies in Poetry lockfile + From base new-sha to target old-sha: + Added **pydantic** (1.10.6) Added **typing-extensions** (4.5.0) Updated **urllib3** (1.26.14 -> 1.26.15) - *(2 added, 0 removed, 1 updated, 4 not changed)*""" - assert format_comment(summary) == dedent(expected_comment) + *(2 added, 0 removed, 1 updated, 4 not changed)* + + Generated by diff-poetry-lock {__version__} + """ + assert ( + format_comment( + summary, + base_commit_hash="new-sha", + target_commit_hash="old-sha", + ) + or "" + ).strip() == dedent(expected_comment).strip() def test_diff_no_changes() -> None: @@ -134,7 +167,7 @@ def test_file_loading_missing_file_base_ref(cfg: Settings) -> None: with requests_mock.Mocker() as m: m.get( f"{cfg.api_url}/repos/{cfg.repository}/contents/{cfg.lockfile_path}?ref={cfg.base_ref}", - headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, + request_headers=GithubApi.request_headers(cfg.token), status_code=404, ) @@ -144,14 +177,10 @@ def test_file_loading_missing_file_base_ref(cfg: Settings) -> None: def test_file_loading_missing_file_head_ref(cfg: Settings, data1: bytes) -> None: with requests_mock.Mocker() as m: - m.get( - f"{cfg.api_url}/repos/{cfg.repository}/contents/{cfg.lockfile_path}?ref={cfg.base_ref}", - headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, - content=data1, - ) + mock_get_file(m, cfg, data1, cfg.base_ref) m.get( f"{cfg.api_url}/repos/{cfg.repository}/contents/{cfg.lockfile_path}?ref={cfg.ref}", - headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, + request_headers=GithubApi.request_headers(cfg.token), status_code=404, ) @@ -171,7 +200,7 @@ def test_e2e_no_diff_existing_comment(cfg: Settings, data1: bytes) -> None: mock_list_comments(m, cfg, comments) m.delete( f"{cfg.api_url}/repos/{cfg.repository}/issues/comments/1337", - headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, + headers=GithubApi.request_headers(cfg.token), ) do_diff(cfg) @@ -187,15 +216,19 @@ def test_e2e_no_diff_inexisting_comment(cfg: Settings, data1: bytes) -> None: def test_e2e_diff_inexisting_comment(cfg: Settings, data1: bytes, data2: bytes) -> None: - summary = format_comment(diff(load_packages(TESTFILE_2), load_packages(TESTFILE_1))) + summary = format_comment( + diff(load_packages(TESTFILE_2), load_packages(TESTFILE_1)), + base_commit_hash="new-sha", + target_commit_hash="old-sha", + ) with requests_mock.Mocker() as m: - mock_get_file(m, cfg, data1, cfg.base_ref) - mock_get_file(m, cfg, data2, cfg.ref) + mock_get_file(m, cfg, data1, cfg.base_ref, resolved_hash="old-sha") + mock_get_file(m, cfg, data2, cfg.ref, resolved_hash="new-sha") mock_list_comments(m, cfg, []) m.post( f"{cfg.api_url}/repos/{cfg.repository}/issues/{cfg.pr_num}/comments", - headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, + headers=GithubApi.request_headers(cfg.token), json={"body": f"{MAGIC_COMMENT_IDENTIFIER}{summary}"}, ) @@ -203,11 +236,15 @@ def test_e2e_diff_inexisting_comment(cfg: Settings, data1: bytes, data2: bytes) def test_e2e_diff_existing_comment_same_data(cfg: Settings, data1: bytes, data2: bytes) -> None: - summary = format_comment(diff(load_packages(TESTFILE_1), load_packages(TESTFILE_2))) + summary = format_comment( + diff(load_packages(TESTFILE_1), load_packages(TESTFILE_2)), + base_commit_hash="new-sha", + target_commit_hash="old-sha", + ) with requests_mock.Mocker() as m: - mock_get_file(m, cfg, data1, cfg.base_ref) - mock_get_file(m, cfg, data2, cfg.ref) + mock_get_file(m, cfg, data1, cfg.base_ref, resolved_hash="old-sha") + mock_get_file(m, cfg, data2, cfg.ref, resolved_hash="new-sha") comments = [ {"body": "foobar", "id": 1334, "user": {"id": 123}}, {"body": "foobar", "id": 1335, "user": {"id": 41898282}}, @@ -219,11 +256,15 @@ def test_e2e_diff_existing_comment_same_data(cfg: Settings, data1: bytes, data2: def test_e2e_diff_existing_comment_different_data(cfg: Settings, data1: bytes, data2: bytes) -> None: - summary = format_comment(diff(load_packages(TESTFILE_1), [])) + summary = format_comment( + diff(load_packages(TESTFILE_1), []), + base_commit_hash="new-sha", + target_commit_hash="old-sha", + ) with requests_mock.Mocker() as m: - mock_get_file(m, cfg, data1, cfg.base_ref) - mock_get_file(m, cfg, data2, cfg.ref) + mock_get_file(m, cfg, data1, cfg.base_ref, resolved_hash="old-sha") + mock_get_file(m, cfg, data2, cfg.ref, resolved_hash="new-sha") comments = [ {"body": "foobar", "id": 1334, "user": {"id": 123}}, {"body": "foobar", "id": 1335, "user": {"id": 41898282}}, @@ -232,13 +273,66 @@ def test_e2e_diff_existing_comment_different_data(cfg: Settings, data1: bytes, d mock_list_comments(m, cfg, comments) m.patch( f"{cfg.api_url}/repos/{cfg.repository}/issues/comments/1337", - headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, + headers=GithubApi.request_headers(cfg.token), json={"body": f"{MAGIC_COMMENT_IDENTIFIER}{summary}"}, ) do_diff(cfg) +def test_e2e_diff_commit_lookup_http_failure_falls_back_to_ref(cfg: Settings, data1: bytes, data2: bytes) -> None: + summary = format_comment( + diff(load_packages(TESTFILE_2), load_packages(TESTFILE_1)), + base_commit_hash=cfg.ref, + target_commit_hash=cfg.base_ref, + ) + + with requests_mock.Mocker() as m: + mock_get_file(m, cfg, data1, cfg.base_ref) + mock_get_file(m, cfg, data2, cfg.ref) + mock_list_comments(m, cfg, []) + m.post( + f"{cfg.api_url}/repos/{cfg.repository}/issues/{cfg.pr_num}/comments", + headers=GithubApi.request_headers(cfg.token), + json={"body": f"{MAGIC_COMMENT_IDENTIFIER}{summary}"}, + ) + + do_diff(cfg) + + +def test_resolve_commit_hash_request_exception_returns_ref(cfg: Settings, monkeypatch: MonkeyPatch) -> None: + api = GithubApi(cfg) + + def raise_timeout(*_args: object, **_kwargs: object) -> Never: + raise requests.Timeout + + monkeypatch.setattr(api.session, "get", raise_timeout) + + assert api.resolve_commit_hash(cfg.ref) == cfg.ref + + +def test_resolve_commit_hash_cache_hit_uses_cached_value(cfg: Settings, data1: bytes) -> None: + api = GithubApi(cfg) + + with requests_mock.Mocker() as m: + mock_get_file(m, cfg, data1, cfg.ref, resolved_hash="cached-sha") + _ = api.get_file(cfg.ref) + + assert api.resolve_commit_hash(cfg.ref) == "cached-sha" + + +def test_resolve_commit_hash_cache_miss_returns_ref(cfg: Settings, monkeypatch: MonkeyPatch) -> None: + api = GithubApi(cfg) + + def fail_if_called(*_args: object, **_kwargs: object) -> Never: + msg = "resolve_commit_hash should not call HTTP" + raise AssertionError(msg) + + monkeypatch.setattr(api.session, "get", fail_if_called) + + assert api.resolve_commit_hash(cfg.ref) == cfg.ref + + def load_file(filename: Path) -> bytes: with filename.open("rb") as f: return f.read() @@ -247,16 +341,23 @@ def load_file(filename: Path) -> bytes: def mock_list_comments(m: Mocker, s: Settings, response_json: list[dict[Any, Any]]) -> None: m.get( f"{s.api_url}/repos/{s.repository}/issues/{s.pr_num}/comments?per_page=100&page=1", - headers={"Authorization": f"Bearer {s.token}", "Accept": "application/vnd.github.raw"}, + request_headers=GithubApi.request_headers(s.token), json=response_json, ) -def mock_get_file(m: Mocker, s: Settings, data: bytes, ref: str) -> None: +def mock_get_file(m: Mocker, s: Settings, data: bytes, ref: str, resolved_hash: str | None = None) -> None: + payload: dict[str, str] = { + "content": base64.b64encode(data).decode("ascii"), + "encoding": "base64", + } + if resolved_hash: + payload["sha"] = resolved_hash + m.get( f"{s.api_url}/repos/{s.repository}/contents/{s.lockfile_path}?ref={ref}", - headers={"Authorization": f"Bearer {s.token}", "Accept": "application/vnd.github.raw"}, - content=data, + request_headers=GithubApi.request_headers(s.token), + json=payload, )