From bdf76be8c844277879a03f7816282411e600553a Mon Sep 17 00:00:00 2001 From: banginji Date: Wed, 25 Feb 2026 23:16:28 -0600 Subject: [PATCH 1/7] Added additional metadata to diff output Added in current and new commit hashes Added the current version of the tool Signed-off-by: banginji --- README.md | 17 ++- diff_poetry_lock/__init__.py | 6 + diff_poetry_lock/github.py | 18 +++ diff_poetry_lock/run_poetry.py | 25 ++++- diff_poetry_lock/test/test_poetry_diff.py | 128 +++++++++++++++++++++- 5 files changed, 186 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index d601221..30383a2 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 + +**Base commit hash (new poetry.lock):** `f4e6ca0f4d67d9bb3f8ab43a89ceca2d0d2be7a1` +**Target commit hash (old poetry.lock):** `a86b84f85d0bb2bf2fca6d6e8c58f2ce6f9e393c` +**diff-poetry-lock version:** `1.0.1` + +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)* +``` ## 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..e1a63e5 100644 --- a/diff_poetry_lock/github.py +++ b/diff_poetry_lock/github.py @@ -101,6 +101,24 @@ def get_file(self, ref: str) -> Response: r.raise_for_status() return r + def resolve_commit_hash(self, ref: str) -> str: + logger.debug("Resolving commit hash for ref {}", ref) + try: + r = self.session.get( + f"{self.s.api_url}/repos/{self.s.repository}/commits/{ref}", + headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"}, + timeout=10, + ) + logger.debug("Response status: {}", r.status_code) + r.raise_for_status() + sha = str(r.json().get("sha", "")).strip() + if sha: + return sha + except Exception: + logger.warning("Failed to resolve 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( diff --git a/diff_poetry_lock/run_poetry.py b/diff_poetry_lock/run_poetry.py index 807dca8..697a726 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,12 @@ 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, + diff_poetry_lock_version: 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")) @@ -89,6 +95,10 @@ def format_comment(packages: list[PackageSummary]) -> str | None: return None comment = f"### Detected {len(added + removed + updated)} changes to dependencies in Poetry lockfile\n\n" + if base_commit_hash and target_commit_hash and diff_poetry_lock_version: + comment += f"**Base commit hash (new poetry.lock):** `{base_commit_hash}`\n" + comment += f"**Target commit hash (old poetry.lock):** `{target_commit_hash}`\n" + comment += f"**diff-poetry-lock version:** `{diff_poetry_lock_version}`\n\n" comment += "\n".join(p.summary_line() for p in added + removed + updated) comment += ( f"\n\n*({len(added)} added, {len(removed)} removed, {len(updated)} updated, {len(not_changed)} not changed)*" @@ -128,7 +138,18 @@ 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, + diff_poetry_lock_version=__version__, + ) 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..eab95e6 100644 --- a/diff_poetry_lock/test/test_poetry_diff.py +++ b/diff_poetry_lock/test/test_poetry_diff.py @@ -4,10 +4,13 @@ from typing import Any import pytest +import requests import requests_mock from _pytest.monkeypatch import MonkeyPatch from requests_mock import Mocker +from diff_poetry_lock import __version__ +from diff_poetry_lock.github import GithubApi from diff_poetry_lock.github import MAGIC_COMMENT_IDENTIFIER from diff_poetry_lock.run_poetry import PackageSummary, diff, do_diff, format_comment, load_packages, main from diff_poetry_lock.settings import GitHubActionsSettings, Settings @@ -77,12 +80,24 @@ def test_diff() -> None: expected_comment = """\ ### Detected 3 changes to dependencies in Poetry lockfile + **Base commit hash (new poetry.lock):** `new-sha` + **Target commit hash (old poetry.lock):** `old-sha` + **diff-poetry-lock version:** `test-version` + 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) + assert ( + format_comment( + summary, + base_commit_hash="new-sha", + target_commit_hash="old-sha", + diff_poetry_lock_version="test-version", + ) + == dedent(expected_comment) + ) def test_diff_2() -> None: @@ -105,12 +120,24 @@ def test_diff_2() -> None: expected_comment = """\ ### Detected 3 changes to dependencies in Poetry lockfile + **Base commit hash (new poetry.lock):** `new-sha` + **Target commit hash (old poetry.lock):** `old-sha` + **diff-poetry-lock version:** `test-version` + 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) + assert ( + format_comment( + summary, + base_commit_hash="new-sha", + target_commit_hash="old-sha", + diff_poetry_lock_version="test-version", + ) + == dedent(expected_comment) + ) def test_diff_no_changes() -> None: @@ -187,11 +214,18 @@ 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", + diff_poetry_lock_version=__version__, + ) 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_commit(m, cfg, cfg.ref, "new-sha") + mock_get_commit(m, cfg, cfg.base_ref, "old-sha") mock_list_comments(m, cfg, []) m.post( f"{cfg.api_url}/repos/{cfg.repository}/issues/{cfg.pr_num}/comments", @@ -203,11 +237,18 @@ 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", + diff_poetry_lock_version=__version__, + ) 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_commit(m, cfg, cfg.ref, "new-sha") + mock_get_commit(m, cfg, cfg.base_ref, "old-sha") comments = [ {"body": "foobar", "id": 1334, "user": {"id": 123}}, {"body": "foobar", "id": 1335, "user": {"id": 41898282}}, @@ -219,11 +260,18 @@ 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", + diff_poetry_lock_version=__version__, + ) 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_commit(m, cfg, cfg.ref, "new-sha") + mock_get_commit(m, cfg, cfg.base_ref, "old-sha") comments = [ {"body": "foobar", "id": 1334, "user": {"id": 123}}, {"body": "foobar", "id": 1335, "user": {"id": 41898282}}, @@ -239,6 +287,68 @@ def test_e2e_diff_existing_comment_different_data(cfg: Settings, data1: bytes, d 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, + diff_poetry_lock_version=__version__, + ) + + with requests_mock.Mocker() as m: + mock_get_file(m, cfg, data1, cfg.base_ref) + mock_get_file(m, cfg, data2, cfg.ref) + m.get( + f"{cfg.api_url}/repos/{cfg.repository}/commits/{cfg.ref}", + headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, + status_code=500, + ) + m.get( + f"{cfg.api_url}/repos/{cfg.repository}/commits/{cfg.base_ref}", + headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, + status_code=500, + ) + 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"}, + 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): + raise requests.Timeout() + + monkeypatch.setattr(api.session, "get", raise_timeout) + + assert api.resolve_commit_hash(cfg.ref) == cfg.ref + + +def test_resolve_commit_hash_invalid_json_returns_ref(cfg: Settings, monkeypatch: MonkeyPatch) -> None: + api = GithubApi(cfg) + + class MockResponse: + status_code = 200 + + @staticmethod + def raise_for_status() -> None: + return None + + @staticmethod + def json() -> dict[str, str]: + msg = "invalid json" + raise ValueError(msg) + + monkeypatch.setattr(api.session, "get", lambda *_args, **_kwargs: MockResponse()) + + assert api.resolve_commit_hash(cfg.ref) == cfg.ref + + def load_file(filename: Path) -> bytes: with filename.open("rb") as f: return f.read() @@ -260,6 +370,14 @@ def mock_get_file(m: Mocker, s: Settings, data: bytes, ref: str) -> None: ) +def mock_get_commit(m: Mocker, s: Settings, ref: str, sha: str) -> None: + m.get( + f"{s.api_url}/repos/{s.repository}/commits/{ref}", + headers={"Authorization": f"Bearer {s.token}", "Accept": "application/vnd.github.raw"}, + json={"sha": sha}, + ) + + def create_settings( repository: str = "user/repo", lockfile_path: str = "poetry.lock", From 45ca911662f45c50c0b470963a31088f2bd2ade6 Mon Sep 17 00:00:00 2001 From: banginji Date: Wed, 25 Feb 2026 23:21:11 -0600 Subject: [PATCH 2/7] Fixed a ruff format error Signed-off-by: banginji --- diff_poetry_lock/test/test_poetry_diff.py | 30 +++++++++-------------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/diff_poetry_lock/test/test_poetry_diff.py b/diff_poetry_lock/test/test_poetry_diff.py index eab95e6..710920e 100644 --- a/diff_poetry_lock/test/test_poetry_diff.py +++ b/diff_poetry_lock/test/test_poetry_diff.py @@ -89,15 +89,12 @@ def test_diff() -> None: Updated **urllib3** (1.26.15 -> 1.26.14) *(0 added, 2 removed, 1 updated, 4 not changed)*""" - assert ( - format_comment( - summary, - base_commit_hash="new-sha", - target_commit_hash="old-sha", - diff_poetry_lock_version="test-version", - ) - == dedent(expected_comment) - ) + assert format_comment( + summary, + base_commit_hash="new-sha", + target_commit_hash="old-sha", + diff_poetry_lock_version="test-version", + ) == dedent(expected_comment) def test_diff_2() -> None: @@ -129,15 +126,12 @@ def test_diff_2() -> None: Updated **urllib3** (1.26.14 -> 1.26.15) *(2 added, 0 removed, 1 updated, 4 not changed)*""" - assert ( - format_comment( - summary, - base_commit_hash="new-sha", - target_commit_hash="old-sha", - diff_poetry_lock_version="test-version", - ) - == dedent(expected_comment) - ) + assert format_comment( + summary, + base_commit_hash="new-sha", + target_commit_hash="old-sha", + diff_poetry_lock_version="test-version", + ) == dedent(expected_comment) def test_diff_no_changes() -> None: From ce0aac8a94bd9d1c6928ec3b0a31814e250cc2f4 Mon Sep 17 00:00:00 2001 From: banginji Date: Wed, 25 Feb 2026 23:26:53 -0600 Subject: [PATCH 3/7] Fixed ruff check error in tests Signed-off-by: banginji --- diff_poetry_lock/test/test_poetry_diff.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/diff_poetry_lock/test/test_poetry_diff.py b/diff_poetry_lock/test/test_poetry_diff.py index 710920e..f87732b 100644 --- a/diff_poetry_lock/test/test_poetry_diff.py +++ b/diff_poetry_lock/test/test_poetry_diff.py @@ -1,7 +1,7 @@ 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 @@ -10,8 +10,7 @@ from requests_mock import Mocker from diff_poetry_lock import __version__ -from diff_poetry_lock.github import GithubApi -from diff_poetry_lock.github import MAGIC_COMMENT_IDENTIFIER +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 @@ -315,8 +314,8 @@ def test_e2e_diff_commit_lookup_http_failure_falls_back_to_ref(cfg: Settings, da def test_resolve_commit_hash_request_exception_returns_ref(cfg: Settings, monkeypatch: MonkeyPatch) -> None: api = GithubApi(cfg) - def raise_timeout(*_args: object, **_kwargs: object): - raise requests.Timeout() + def raise_timeout(*_args: object, **_kwargs: object) -> Never: + raise requests.Timeout monkeypatch.setattr(api.session, "get", raise_timeout) From 7222aeef5100999b212c8b184a10bcff95e30dc7 Mon Sep 17 00:00:00 2001 From: banginji Date: Mon, 2 Mar 2026 19:42:45 -0600 Subject: [PATCH 4/7] Made fixes based on review comments 1- Modified commit hash lookup to be done in the get_file api call 2- Altered diff output format 3- Wrapped the api headers into a function to be reused instead of the headers dict Signed-off-by: banginji --- README.md | 8 +- diff_poetry_lock/github.py | 54 +++++---- diff_poetry_lock/run_poetry.py | 21 ++-- diff_poetry_lock/test/test_poetry_diff.py | 134 +++++++++------------- 4 files changed, 98 insertions(+), 119 deletions(-) diff --git a/README.md b/README.md index 30383a2..cfecbc3 100644 --- a/README.md +++ b/README.md @@ -18,19 +18,15 @@ This friction complicates the responsible acceptance of pull requests that chang ```markdown ### Detected 6 changes to dependencies in Poetry lockfile - -**Base commit hash (new poetry.lock):** `f4e6ca0f4d67d9bb3f8ab43a89ceca2d0d2be7a1` -**Target commit hash (old poetry.lock):** `a86b84f85d0bb2bf2fca6d6e8c58f2ce6f9e393c` -**diff-poetry-lock version:** `1.0.1` - +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\n\n ``` ## Usage diff --git a/diff_poetry_lock/github.py b/diff_poetry_lock/github.py index e1a63e5..bfb5f8c 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,46 +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: - logger.debug("Resolving commit hash for ref {}", ref) - try: - r = self.session.get( - f"{self.s.api_url}/repos/{self.s.repository}/commits/{ref}", - headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"}, - timeout=10, - ) - logger.debug("Response status: {}", r.status_code) - r.raise_for_status() - sha = str(r.json().get("sha", "")).strip() - if sha: - return sha - except Exception: - logger.warning("Failed to resolve commit hash for ref {}, falling back to ref", ref) + 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() @@ -140,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) @@ -155,6 +158,9 @@ 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 {"Authorization": f"token {self.s.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 697a726..3f28ece 100644 --- a/diff_poetry_lock/run_poetry.py +++ b/diff_poetry_lock/run_poetry.py @@ -84,7 +84,6 @@ def format_comment( packages: list[PackageSummary], base_commit_hash: str | None = None, target_commit_hash: str | None = None, - diff_poetry_lock_version: 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")) @@ -94,24 +93,25 @@ def format_comment( if len(added + removed + updated) == 0: return None - comment = f"### Detected {len(added + removed + updated)} changes to dependencies in Poetry lockfile\n\n" - if base_commit_hash and target_commit_hash and diff_poetry_lock_version: - comment += f"**Base commit hash (new poetry.lock):** `{base_commit_hash}`\n" - comment += f"**Target commit hash (old poetry.lock):** `{target_commit_hash}`\n" - comment += f"**diff-poetry-lock version:** `{diff_poetry_lock_version}`\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"Generated 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)) @@ -148,7 +148,6 @@ def do_diff(settings: Settings) -> None: packages, base_commit_hash=base_commit_hash, target_commit_hash=target_commit_hash, - diff_poetry_lock_version=__version__, ) if summary: diff --git a/diff_poetry_lock/test/test_poetry_diff.py b/diff_poetry_lock/test/test_poetry_diff.py index f87732b..a8b6976 100644 --- a/diff_poetry_lock/test/test_poetry_diff.py +++ b/diff_poetry_lock/test/test_poetry_diff.py @@ -1,3 +1,4 @@ +import base64 from operator import attrgetter from pathlib import Path from textwrap import dedent @@ -76,24 +77,24 @@ def test_diff() -> None: ] assert summary == expected - expected_comment = """\ + expected_comment = f"""\ ### Detected 3 changes to dependencies in Poetry lockfile - **Base commit hash (new poetry.lock):** `new-sha` - **Target commit hash (old poetry.lock):** `old-sha` - **diff-poetry-lock version:** `test-version` + 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, - base_commit_hash="new-sha", - target_commit_hash="old-sha", - diff_poetry_lock_version="test-version", - ) == 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_diff_2() -> None: @@ -113,24 +114,24 @@ def test_diff_2() -> None: ] assert summary == expected - expected_comment = """\ + expected_comment = f"""\ ### Detected 3 changes to dependencies in Poetry lockfile - **Base commit hash (new poetry.lock):** `new-sha` - **Target commit hash (old poetry.lock):** `old-sha` - **diff-poetry-lock version:** `test-version` + 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, - base_commit_hash="new-sha", - target_commit_hash="old-sha", - diff_poetry_lock_version="test-version", - ) == 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: @@ -154,7 +155,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={"Authorization": f"token {cfg.token}", "Accept": "application/vnd.github+json"}, status_code=404, ) @@ -164,14 +165,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={"Authorization": f"token {cfg.token}", "Accept": "application/vnd.github+json"}, status_code=404, ) @@ -211,14 +208,11 @@ def test_e2e_diff_inexisting_comment(cfg: Settings, data1: bytes, data2: bytes) diff(load_packages(TESTFILE_2), load_packages(TESTFILE_1)), base_commit_hash="new-sha", target_commit_hash="old-sha", - diff_poetry_lock_version=__version__, ) 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_commit(m, cfg, cfg.ref, "new-sha") - mock_get_commit(m, cfg, cfg.base_ref, "old-sha") + 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", @@ -234,14 +228,11 @@ def test_e2e_diff_existing_comment_same_data(cfg: Settings, data1: bytes, data2: diff(load_packages(TESTFILE_1), load_packages(TESTFILE_2)), base_commit_hash="new-sha", target_commit_hash="old-sha", - diff_poetry_lock_version=__version__, ) 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_commit(m, cfg, cfg.ref, "new-sha") - mock_get_commit(m, cfg, cfg.base_ref, "old-sha") + 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}}, @@ -257,14 +248,11 @@ def test_e2e_diff_existing_comment_different_data(cfg: Settings, data1: bytes, d diff(load_packages(TESTFILE_1), []), base_commit_hash="new-sha", target_commit_hash="old-sha", - diff_poetry_lock_version=__version__, ) 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_commit(m, cfg, cfg.ref, "new-sha") - mock_get_commit(m, cfg, cfg.base_ref, "old-sha") + 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}}, @@ -285,22 +273,11 @@ def test_e2e_diff_commit_lookup_http_failure_falls_back_to_ref(cfg: Settings, da diff(load_packages(TESTFILE_2), load_packages(TESTFILE_1)), base_commit_hash=cfg.ref, target_commit_hash=cfg.base_ref, - diff_poetry_lock_version=__version__, ) with requests_mock.Mocker() as m: mock_get_file(m, cfg, data1, cfg.base_ref) mock_get_file(m, cfg, data2, cfg.ref) - m.get( - f"{cfg.api_url}/repos/{cfg.repository}/commits/{cfg.ref}", - headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, - status_code=500, - ) - m.get( - f"{cfg.api_url}/repos/{cfg.repository}/commits/{cfg.base_ref}", - headers={"Authorization": f"Bearer {cfg.token}", "Accept": "application/vnd.github.raw"}, - status_code=500, - ) mock_list_comments(m, cfg, []) m.post( f"{cfg.api_url}/repos/{cfg.repository}/issues/{cfg.pr_num}/comments", @@ -322,22 +299,24 @@ def raise_timeout(*_args: object, **_kwargs: object) -> Never: assert api.resolve_commit_hash(cfg.ref) == cfg.ref -def test_resolve_commit_hash_invalid_json_returns_ref(cfg: Settings, monkeypatch: MonkeyPatch) -> None: +def test_resolve_commit_hash_cache_hit_uses_cached_value(cfg: Settings, data1: bytes) -> None: api = GithubApi(cfg) - class MockResponse: - status_code = 200 + 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" + - @staticmethod - def raise_for_status() -> None: - return None +def test_resolve_commit_hash_cache_miss_returns_ref(cfg: Settings, monkeypatch: MonkeyPatch) -> None: + api = GithubApi(cfg) - @staticmethod - def json() -> dict[str, str]: - msg = "invalid json" - raise ValueError(msg) + 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", lambda *_args, **_kwargs: MockResponse()) + monkeypatch.setattr(api.session, "get", fail_if_called) assert api.resolve_commit_hash(cfg.ref) == cfg.ref @@ -350,24 +329,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={"Authorization": f"token {s.token}", "Accept": "application/vnd.github+json"}, json=response_json, ) -def mock_get_file(m: Mocker, s: Settings, data: bytes, ref: str) -> None: - 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, - ) - +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 -def mock_get_commit(m: Mocker, s: Settings, ref: str, sha: str) -> None: m.get( - f"{s.api_url}/repos/{s.repository}/commits/{ref}", - headers={"Authorization": f"Bearer {s.token}", "Accept": "application/vnd.github.raw"}, - json={"sha": sha}, + f"{s.api_url}/repos/{s.repository}/contents/{s.lockfile_path}?ref={ref}", + request_headers={"Authorization": f"token {s.token}", "Accept": "application/vnd.github+json"}, + json=payload, ) From a3170a14f2758f62ac2f8ae7a565055ab12a4a6e Mon Sep 17 00:00:00 2001 From: banginji Date: Mon, 2 Mar 2026 19:48:46 -0600 Subject: [PATCH 5/7] Fixed an error pointed out by ruff format Signed-off-by: banginji --- diff_poetry_lock/test/test_poetry_diff.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/diff_poetry_lock/test/test_poetry_diff.py b/diff_poetry_lock/test/test_poetry_diff.py index a8b6976..49b1946 100644 --- a/diff_poetry_lock/test/test_poetry_diff.py +++ b/diff_poetry_lock/test/test_poetry_diff.py @@ -93,7 +93,8 @@ def test_diff() -> None: summary, base_commit_hash="new-sha", target_commit_hash="old-sha", - ) or "" + ) + or "" ).strip() == dedent(expected_comment).strip() @@ -130,7 +131,8 @@ def test_diff_2() -> None: summary, base_commit_hash="new-sha", target_commit_hash="old-sha", - ) or "" + ) + or "" ).strip() == dedent(expected_comment).strip() From f1daf32dcd51f46980c8cb578ca77a33c28db3ec Mon Sep 17 00:00:00 2001 From: banginji Date: Tue, 3 Mar 2026 09:53:09 -0600 Subject: [PATCH 6/7] Updates based on latest review comments 1- Created a static method for request_headers and reused it across 2- Updated auth header to now use Bearer keyword instead of token keyword 3- Fixed diff output comment Signed-off-by: banginji --- diff_poetry_lock/github.py | 6 ++++- diff_poetry_lock/run_poetry.py | 2 +- diff_poetry_lock/test/test_poetry_diff.py | 30 +++++++++++++++-------- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/diff_poetry_lock/github.py b/diff_poetry_lock/github.py index bfb5f8c..fb60628 100644 --- a/diff_poetry_lock/github.py +++ b/diff_poetry_lock/github.py @@ -159,7 +159,11 @@ def find_pr_for_branch(self, branch_ref: str) -> str: return "" def api_headers(self) -> dict[str, str]: - return {"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"} + 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: diff --git a/diff_poetry_lock/run_poetry.py b/diff_poetry_lock/run_poetry.py index 3f28ece..18dbf17 100644 --- a/diff_poetry_lock/run_poetry.py +++ b/diff_poetry_lock/run_poetry.py @@ -103,7 +103,7 @@ def format_comment( f"\n\n*({len(added)} added, {len(removed)} removed, {len(updated)} updated, {len(not_changed)} not changed)*" ) if __version__: - comment += f"Generated by diff-poetry-lock {__version__}\n\n" + comment += f"\n\nGenerated by diff-poetry-lock {__version__}\n\n" return comment diff --git a/diff_poetry_lock/test/test_poetry_diff.py b/diff_poetry_lock/test/test_poetry_diff.py index 49b1946..6a6887b 100644 --- a/diff_poetry_lock/test/test_poetry_diff.py +++ b/diff_poetry_lock/test/test_poetry_diff.py @@ -86,7 +86,9 @@ def test_diff() -> None: Removed **typing-extensions** (4.5.0) Updated **urllib3** (1.26.15 -> 1.26.14) - *(0 added, 2 removed, 1 updated, 4 not changed)*Generated by diff-poetry-lock {__version__} + *(0 added, 2 removed, 1 updated, 4 not changed)* + + Generated by diff-poetry-lock {__version__} """ assert ( format_comment( @@ -98,6 +100,12 @@ def test_diff() -> None: ).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: old = load_packages(TESTFILE_2) new = load_packages(TESTFILE_1) @@ -124,7 +132,9 @@ def test_diff_2() -> None: Added **typing-extensions** (4.5.0) Updated **urllib3** (1.26.14 -> 1.26.15) - *(2 added, 0 removed, 1 updated, 4 not changed)*Generated by diff-poetry-lock {__version__} + *(2 added, 0 removed, 1 updated, 4 not changed)* + + Generated by diff-poetry-lock {__version__} """ assert ( format_comment( @@ -157,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}", - request_headers={"Authorization": f"token {cfg.token}", "Accept": "application/vnd.github+json"}, + request_headers=GithubApi.request_headers(cfg.token), status_code=404, ) @@ -170,7 +180,7 @@ def test_file_loading_missing_file_head_ref(cfg: Settings, data1: bytes) -> None 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}", - request_headers={"Authorization": f"token {cfg.token}", "Accept": "application/vnd.github+json"}, + request_headers=GithubApi.request_headers(cfg.token), status_code=404, ) @@ -190,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) @@ -218,7 +228,7 @@ def test_e2e_diff_inexisting_comment(cfg: Settings, data1: bytes, data2: bytes) 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}"}, ) @@ -263,7 +273,7 @@ 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}"}, ) @@ -283,7 +293,7 @@ def test_e2e_diff_commit_lookup_http_failure_falls_back_to_ref(cfg: Settings, da 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}"}, ) @@ -331,7 +341,7 @@ 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", - request_headers={"Authorization": f"token {s.token}", "Accept": "application/vnd.github+json"}, + request_headers=GithubApi.request_headers(s.token), json=response_json, ) @@ -346,7 +356,7 @@ def mock_get_file(m: Mocker, s: Settings, data: bytes, ref: str, resolved_hash: m.get( f"{s.api_url}/repos/{s.repository}/contents/{s.lockfile_path}?ref={ref}", - request_headers={"Authorization": f"token {s.token}", "Accept": "application/vnd.github+json"}, + request_headers=GithubApi.request_headers(s.token), json=payload, ) From d94df0bc058fcab9b565d6af8f87638dc325e74f Mon Sep 17 00:00:00 2001 From: banginji Date: Tue, 3 Mar 2026 10:02:21 -0600 Subject: [PATCH 7/7] Updated readme to the correctly formatted example output Signed-off-by: banginji --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cfecbc3..9e3b1f1 100644 --- a/README.md +++ b/README.md @@ -18,15 +18,19 @@ This friction complicates the responsible acceptance of pull requests that chang ```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\n\n + +Generated by diff-poetry-lock 1.0.1 ``` ## Usage