#160 - Update to latest version of PyGitHub #161
Conversation
- Updated version of PyGitHub to latest one. Required update of code and unit tests.
WalkthroughUpdates PyGithub to 2.8.1 and adapts rate limit handling to the new API by switching from core-based to rate-based fields, using RateLimitOverview. Adjusts sleep logic to handle past reset times and adds buffer. Updates tests and fixtures to the new attribute paths and datetime-based reset. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant RateLimiter as GithubRateLimiter
participant PyGithub as PyGithub Client
Caller->>RateLimiter: call wrapped_method(...)
RateLimiter->>PyGithub: get_rate_limit()
PyGithub-->>RateLimiter: RateLimitOverview (rate.remaining, rate.reset: datetime)
alt remaining > 0
RateLimiter->>RateLimiter: proceed without sleep
else remaining == 0
RateLimiter->>RateLimiter: now = time.time()
RateLimiter->>RateLimiter: sleep_time = reset.timestamp - now
opt reset in past
RateLimiter->>RateLimiter: increment reset by 3600s until future
end
RateLimiter->>RateLimiter: sleep(sleep_time + 5s buffer)
end
RateLimiter-->>Caller: execute wrapped_method and return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(None found) Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
miroslavpojer
deleted the
feature/160-Update-to-latest-version-of-PyGitHub
branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
tests/test_miner.py (1)
71-71: Unify rate-limit attribute usage (rate vs core).Elsewhere in this file you still use core.remaining; here it’s rate.remaining. Pick one to avoid brittle mocks. Given PyGithub documents core/search/graphql on RateLimit, prefer core. (pygithub.readthedocs.io)
Apply:
- mock_rate_limit.rate.remaining = 1000 + mock_rate_limit.core.remaining = 1000Also applies to: 98-98
tests/test_release_notes_generator.py (1)
36-38: Prefer RateLimit.core over rate for stability.PyGithub’s public docs show core/search/graphql on RateLimit; the top-level rate field isn’t guaranteed. Align these mocks with core to reduce breakage risk. (pygithub.readthedocs.io)
Apply:
- mock_rate_limit.rate.remaining = 10 - mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 + mock_rate_limit.core.remaining = 10 + mock_rate_limit.core.reset.timestamp.return_value = time.time() + 3600- mock_rate_limit.rate.remaining = 1000 + mock_rate_limit.core.remaining = 1000- mock_rate_limit.rate.remaining = 1000 + mock_rate_limit.core.remaining = 1000- mock_rate_limit.rate.remaining = 1000 + mock_rate_limit.core.remaining = 1000Also applies to: 72-72, 118-118, 170-170
release_notes_generator/utils/github_rate_limiter.py (2)
24-24: Remove unused import.
“cast” isn’t used.-from typing import Optional, Callable, Any, cast +from typing import Optional, Callable, Any
47-49: Use RateLimit.core; avoid ambiguous “rate”.PyGithub exposes core/search/graphql on RateLimit. Using core is explicit and documented; some environments may not expose a top-level rate property. (pygithub.readthedocs.io)
- rate_limit_overview = self.github_client.get_rate_limit() - remaining_calls = rate_limit_overview.rate.remaining - reset_time = rate_limit_overview.rate.reset.timestamp() + core = self.github_client.get_rate_limit().core + remaining_calls = core.remaining + reset_time = core.reset.timestamp()Since PyGithub ≥2.1.0 ships built-in throttling and retries, consider relying on Github(seconds_between_requests=..., seconds_between_writes=..., retry=GithubRetry()) and keep this decorator only for extra safeguards. (github.com)
I can wire a config toggle and update tests if you want to lean on PyGithub’s throttling.
tests/release_notes/test_record_factory.py (1)
180-182: Standardize on RateLimit.core across tests.Replace rate.* with core.* for consistency with PyGithub’s documented API. (pygithub.readthedocs.io)
- mock_rate_limit.rate.remaining = 10 - mock_rate_limit.rate.reset.timestamp.return_value = time.time() + 3600 + mock_rate_limit.core.remaining = 10 + mock_rate_limit.core.reset.timestamp.return_value = time.time() + 3600(Apply similarly to the other two hunks in this file.)
Also applies to: 274-276, 309-311
tests/utils/test_github_rate_limiter.py (1)
24-24: Align test fixtures with core. and the corrected conftest.*After fixing conftest to return a RateLimit-like object with core, adapt these lines.
- mock_rate_limiter.rate.remaining = 1 + mock_rate_limiter.core.remaining = 1- mock_rate_limiter.rate.remaining = 1 - mock_rate_limiter.rate.reset = datetime.now() + timedelta(hours=1) + mock_rate_limiter.core.remaining = 1 + mock_rate_limiter.core.reset = datetime.now() + timedelta(hours=1)Also applies to: 53-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
release_notes_generator/utils/github_rate_limiter.py(3 hunks)requirements.txt(1 hunks)tests/conftest.py(3 hunks)tests/release_notes/test_record_factory.py(3 hunks)tests/test_miner.py(2 hunks)tests/test_release_notes_generator.py(4 hunks)tests/utils/test_github_rate_limiter.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/utils/test_github_rate_limiter.py (1)
tests/conftest.py (2)
rate_limiter(106-109)mock_rate_limiter(113-121)
tests/conftest.py (1)
release_notes_generator/utils/github_rate_limiter.py (1)
GithubRateLimiter(30-76)
🔇 Additional comments (1)
requirements.txt (1)
13-13: PyGithub bump looks good; confirm "latest" intent.2.8.1 is currently the latest on PyPI (uploaded Sep 2, 2025). If your goal is “latest,” this satisfies Issue #160. (pypi.org)
If you want a quick check in CI to catch future outdated pins, I can add a small job that compares the pinned version to PyPI’s JSON.
| from github.PullRequest import PullRequest | ||
| from github.Rate import Rate | ||
| from github.RateLimit import RateLimit | ||
| from github.RateLimitOverview import RateLimitOverview |
There was a problem hiding this comment.
Invalid import: RateLimitOverview doesn’t exist in PyGithub.
Docs and sources reference github.RateLimit.RateLimit; there’s no RateLimitOverview class to import. This will fail at import time. (pygithub.readthedocs.io, github.com)
-from github.RateLimitOverview import RateLimitOverview
+# No RateLimitOverview in PyGithub; remove this import📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from github.RateLimitOverview import RateLimitOverview | |
| # No RateLimitOverview in PyGithub; remove this import |
🤖 Prompt for AI Agents
In tests/conftest.py around line 29, the import "from github.RateLimitOverview
import RateLimitOverview" is invalid because PyGithub exposes RateLimit as
github.RateLimit.RateLimit; replace the import with the correct module/class
(e.g., "from github.RateLimit import RateLimit") and update any subsequent
references to use RateLimit (or the appropriate object returned by the library)
so tests import the real PyGithub class and do not fail at import time.
| mock_rate.reset = datetime.now() + timedelta(hours=1) | ||
|
|
||
| mock = mocker.Mock(spec=GithubRateLimiter) | ||
| mock.core = mock_core | ||
| mock.core.remaining = 10 | ||
| mock.rate = mock_rate | ||
| mock.rate.remaining = 10 | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Mock the right spec and prefer core.
Return a RateLimit-like object from get_rate_limit(), not a GithubRateLimiter mock. Also align on core.*.
- mock_rate = mocker.Mock(spec=Rate)
- mock_rate.reset = datetime.now() + timedelta(hours=1)
-
- mock = mocker.Mock(spec=GithubRateLimiter)
- mock.rate = mock_rate
- mock.rate.remaining = 10
+ mock_rate = mocker.Mock(spec=Rate)
+ mock_rate.reset = datetime.now() + timedelta(hours=1)
+
+ # This object mimics github.RateLimit.RateLimit
+ from github.RateLimit import RateLimit # optional: type hint only
+ mock = mocker.Mock() # don’t use GithubRateLimiter as spec here
+ mock.core = mock_rate
+ mock.core.remaining = 10Run to catch stray usages:
🏁 Script executed:
#!/bin/bash
rg -nP '\.get_rate_limit\(\)\.rate\b|\.rate\.(remaining|reset)\b'Length of output: 1782
Mock core instead of rate across the test suite
Replace all occurrences of mock*.rate.* with mock*.core.* when mocking the GitHub RateLimit:
- Apply the diff in
tests/conftest.py(usemock.coreper the snippet). - Propagate the change in:
tests/test_release_notes_generator.py(lines 36–37, 72, 118, 170)tests/test_miner.py(lines 71, 98)tests/utils/test_github_rate_limiter.py(lines 24, 53–54)tests/release_notes/test_record_factory.py(lines 180–181, 274–275, 309–310)
- Update
release_notes_generator/utils/github_rate_limiter.pyto consume.coreinstead of.rate.
Ensure get_rate_limit() returns a RateLimit-like object on .core, not a GithubRateLimiter mock.
🤖 Prompt for AI Agents
In tests/conftest.py around lines 115–120, the tests currently mock
GithubRateLimiter attributes on .rate but the code expects a RateLimit-like
object under .core; change mock_rate assignment to set mock.core = mock_rate and
replace uses of mock.rate.* with mock.core.*; propagate this replacement in
tests/test_release_notes_generator.py (lines 36–37, 72, 118, 170),
tests/test_miner.py (lines 71, 98), tests/utils/test_github_rate_limiter.py
(lines 24, 53–54), and tests/release_notes/test_record_factory.py (lines
180–181, 274–275, 309–310) so all tests reference mock*.core.*; finally update
release_notes_generator/utils/github_rate_limiter.py to consume .core from
get_rate_limit() (ensure get_rate_limit() returns a RateLimit-like object
available on .core, not a GithubRateLimiter mock) so production code and tests
align.
1 participant
Release Notes:
Summary by CodeRabbit
Fixes #160