AbsaOSS / AbsaOSS/generate-release-notes

Tech-debt: Since-time mode: `since=None` can crash `get_issues()`, and API failures silently degrade to empty results

未關閉
#338 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
主要語言
Python
星號
13
分支
0
平均合併
6 小時 50 分鐘
30 天內合併 PR
7

描述

Found while reviewing PR #337 (Copilot comments on the pagination-safety refactor, since reverted from that PR as out of scope for #335).

## Problem 1 — `since=None` can raise inside PyGithub

In `DataMiner._get_issues`, `data.since` is `Optional[datetime]` and gets passed straight through to `Repository.get_issues(..., since=data.since)`. PyGithub's implementation asserts `is_optional(since, datetime)`, i.e. `isinstance(v, NotSetType) or isinstance(v, datetime)` — explicitly passing `None` satisfies neither, so `get_issues(since=None)` raises `AssertionError: None` instead of falling back to its default. Today this is masked because the call goes through `_safe_call`'s untyped wrapper, and is *also* practically unreachable in this exact spot (a GitHub release always has `created_at`) — but it's a latent crash risk, and the same pattern (attribute typed `Optional[datetime]`, passed unconditionally as a kwarg) could easily recur elsewhere.

Fix: only pass `since` when it's not `None` (omit the kwarg so PyGithub's `NotSet` default applies), instead of suppressing the type checker.

## Problem 2 — `_safe_call` failures silently produce incomplete results in since-time mode

`_safe_call` catches `GithubException`/network errors and returns `None`. In compare mode, a `None` result is treated as fatal (`sys.exit(1)`) — e.g. `comparison is None`. In since-time mode, several fetches (`repo.get_pulls`, `repo.get_commits`, `repo.get_releases`, `repo.get_issues`) instead do `... or []`, so a transient API failure silently becomes "0 PRs found" / "0 commits found" / "treat this as the first release" and the run completes "successfully" with incomplete or wrong release notes.

Decide and apply consistently: should a `_safe_call` failure during since-time mining be fatal (matching compare mode), or is partial/degraded output acceptable with a loud warning? Whichever is chosen, apply it uniformly across all the since-time-mode fetches instead of the current ad hoc mix of "crash" (pre-existing, for some paths) and "silently continue" (introduced in #337 for others).

Update - see latest [post from copilot review](https://github.com/AbsaOSS/generate-release-notes/pull/337#pullrequestreview-5007123333)

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。