Skip to content

Fix "ingest push" for GitHub repos by reading the compare API as a dict#9625

Draft
hneiva wants to merge 1 commit into
mozilla:masterfrom
hneiva:hneiva/fix-ingest-compare
Draft

Fix "ingest push" for GitHub repos by reading the compare API as a dict#9625
hneiva wants to merge 1 commit into
mozilla:masterfrom
hneiva:hneiva/fix-ingest-compare

Conversation

@hneiva

@hneiva hneiva commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Bug 2009865 switched github.compare_shas() from the compare REST endpoint (which returns a dict) to PyGithub (a list of commit objects) and updated the Pulse push loader to match — but left the ingest management command's query_data() doing dict access on the result:

compare_response.get("merge_base_commit")
compare_response["commits"]

The GitHub compare API always returns a merge_base_commit, so query_data hit .get() on a list and raised AttributeError immediately — meaning ./manage.py ingest push <github-repo> has been broken since that change.

Call the compare REST endpoint directly via fetch_api() for both compare requests in query_data, restoring the dict shape it expects. The Pulse push loader is unaffected and keeps using compare_shas().

Add a regression test for query_data's dict handling; this code path had no test coverage, which is how the regression went unnoticed.

Bug 2009865 switched github.compare_shas() from the compare REST endpoint
(which returns a dict) to PyGithub (a list of commit objects) and updated
the Pulse push loader to match — but left the `ingest` management
command's query_data() doing dict access on the result:

    compare_response.get("merge_base_commit")
    compare_response["commits"]

The GitHub compare API always returns a merge_base_commit, so query_data
hit `.get()` on a list and raised AttributeError immediately — meaning
`./manage.py ingest push <github-repo>` has been broken since that change.

Call the compare REST endpoint directly via fetch_api() for both compare
requests in query_data, restoring the dict shape it expects. The Pulse
push loader is unaffected and keeps using compare_shas().

Add a regression test for query_data's dict handling; this code path had
no test coverage, which is how the regression went unnoticed.
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.

1 participant