Skip to content

Fix: Handle /releases/tags/{tag} URL conversion to canonical form#3468

Closed
SebastienSyd wants to merge 2 commits intoPyGithub:mainfrom
SebastienSyd:fix/release-tags-url-issue-3467
Closed

Fix: Handle /releases/tags/{tag} URL conversion to canonical form#3468
SebastienSyd wants to merge 2 commits intoPyGithub:mainfrom
SebastienSyd:fix/release-tags-url-issue-3467

Conversation

@SebastienSyd
Copy link
Copy Markdown
Contributor

Description

Fixes #3467

When a release is fetched by tag name using repo.get_release("v1.0"), the URL was incorrectly set to /releases/tags/TAG_NAME instead of /releases/ID, which caused subsequent API calls like get_assets() to fail with a 404 error.

Root Cause

The issue was introduced in v2.9.0 with lazy loading for releases (PR #3403). The __postProcess() method in Requester.py was setting the object's URL to the GET request URL when the response didn't contain a url field. For tag-based lookups (/releases/tags/{tag}), this resulted in a non-canonical URL being stored.

Solution

Updated __postProcess() in Requester.py to:

  1. Detect when the response contains an id field AND the URL contains /tags/ (GitHub's name-based lookup pattern)
  2. Reconstruct the URL with the canonical ID-based form: /releases/{id}

This approach handles GitHub's API pattern for name-based lookups generically.

Changes

  • github/Requester.py: Added logic to reconstruct URLs containing /tags/ with the canonical ID-based form
  • tests/GitRelease.py: Added testGetAssetsByTag() to verify releases fetched by tag can retrieve assets without 404 errors

Testing

The new test testGetAssetsByTag() validates that:

  • A release can be fetched by tag name
  • Assets can be retrieved from that release without 404 errors
  • The correct asset data is returned

Impact

  • Fixes the regression introduced in v2.9.0
  • Maintains compatibility with lazy loading
  • Doesn't affect other endpoints or non-tag-based lookups

When a release is fetched by tag name using repo.get_release('v1.0'),
the URL was set to /releases/tags/TAG_NAME instead of /releases/ID,
which caused subsequent API calls like get_assets() to fail with 404.

This fix updates __postProcess() in Requester.py to detect when:
1. The response contains an 'id' field
2. The URL contains '/tags/' (GitHub's name-based lookup pattern)

In these cases, it reconstructs the URL with the canonical ID-based form.

Also added a test case testGetAssetsByTag() to verify the fix.

Fixes PyGithub#3467
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 23, 2026

Test Results

     8 files  ±0       8 suites  ±0   4m 27s ⏱️ +7s
 1 108 tests +1   1 108 ✅ +1  0 💤 ±0  0 ❌ ±0 
14 056 runs  +8  14 055 ✅ +8  1 💤 ±0  0 ❌ ±0 

Results for commit 772b2c9. ± Comparison against base commit 3a17ecf.

♻️ This comment has been updated with latest results.

The test requires replay data to simulate API responses without
making real API calls during testing.
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.05%. Comparing base (7de2644) to head (772b2c9).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
github/Requester.py 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3468      +/-   ##
==========================================
+ Coverage   97.00%   97.05%   +0.04%     
==========================================
  Files         172      176       +4     
  Lines       18934    19797     +863     
==========================================
+ Hits        18366    19213     +847     
- Misses        568      584      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem is not Requester reusing the URL here, but Release using the release-by-tag URL as the lazy URL in the first place.

Comment thread github/Requester.py
data["url"] = href
else:
data["url"] = url
# GitHub API provides name-based lookup shortcuts (e.g., /releases/tags/{tag})
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very class-specific logic in a very general place.

Comment thread tests/GitRelease.py
self.assertTrue(asset is not None)
self.assertEqual(asset.id, asset_id)

def testGetAssetsByTag(self):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test does not reproduce the issue. It already passes without your code changes in Requester.py.

EnricoMi added a commit that referenced this pull request Mar 24, 2026
Getting a release by tag cannot return a lazy release. The URL that is
used to fetch the release by tag cannot be used for subsequent requests
(like getting assets). The URL first has to be turned into the URL that
contains the release id. This requires an API call.

Supersedes #3468. Fixes #3467.

Co-authored-by: Sebastien NICOT <[email protected]>
@EnricoMi
Copy link
Copy Markdown
Collaborator

Closing in favour of #3469.

@EnricoMi EnricoMi closed this Mar 24, 2026
Copilot AI pushed a commit that referenced this pull request Mar 26, 2026
Getting a release by tag cannot return a lazy release. The URL that is
used to fetch the release by tag cannot be used for subsequent requests
(like getting assets). The URL first has to be turned into the URL that
contains the release id. This requires an API call.

Supersedes #3468. Fixes #3467.

Co-authored-by: Sebastien NICOT <[email protected]>
Co-authored-by: EnricoMi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: get_assets() fails with 404 when release is fetched by tag in v2.9.0

2 participants