Skip to content

Fix KeyError when GitHub repo move response lacks full_name#3630

Draft
HimasreeKolathur24 wants to merge 1 commit intoaugurlabs:mainfrom
HimasreeKolathur24:fix/3621-handle-missing-full-name
Draft

Fix KeyError when GitHub repo move response lacks full_name#3630
HimasreeKolathur24 wants to merge 1 commit intoaugurlabs:mainfrom
HimasreeKolathur24:fix/3621-handle-missing-full-name

Conversation

@HimasreeKolathur24
Copy link
Copy Markdown
Contributor

This PR fixes a crash in the GitHub repository move detection task that occurred
when the GitHub API redirect response did not include the full_name field.

Previously, the code assumed full_name was always present and raised a
KeyError when it was missing. This PR makes the logic more defensive by:

  • Safely checking for the presence of full_name
  • Logging a warning when it is missing
  • Gracefully handling the repo move without breaking collection

This improves robustness when GitHub returns partial or unexpected redirect
metadata.

This PR fixes

Fixes #3621

Notes for Reviewers

  • This is a defensive bug fix only
  • No schema, migration, or data changes
  • Existing behavior is unchanged when full_name is present
  • Prevents GitHub collection tasks from failing due to incomplete API responses

Signed commits

  • Yes, I signed my commits.

@HimasreeKolathur24 HimasreeKolathur24 force-pushed the fix/3621-handle-missing-full-name branch from fbbbfe8 to 4125cd4 Compare January 27, 2026 17:13
Copy link
Copy Markdown
Collaborator

@MoralCode MoralCode left a comment

Choose a reason for hiding this comment

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

Overall I think the core change here is a good start, but

  1. id like to understand the reason why the API can return data without full_name as that will help inform how we handle things
  2. there are a lot of unrelated formatting changes in this PR that make me a little nervous about the possibility of accidentally changing things. Did those come from your editor? or were they somehow derived from our own existing linter rules?

Comment thread augur/tasks/github/detect_move/core.py
Comment thread augur/tasks/github/detect_move/core.py Outdated
Comment thread augur/tasks/github/detect_move/core.py Outdated
Comment thread augur/tasks/github/detect_move/core.py Outdated
Comment thread augur/tasks/github/detect_move/core.py
Signed-off-by: Himasree Kolathur <himaa@Himasrees-MacBook-Air.local>
@HimasreeKolathur24 HimasreeKolathur24 force-pushed the fix/3621-handle-missing-full-name branch from ec3f78b to 4b267c6 Compare February 6, 2026 17:32
@HimasreeKolathur24
Copy link
Copy Markdown
Contributor Author

@MoralCode,
I’ve addressed the feedback and pushed updates. Thanks for taking the time to review.

@MoralCode
Copy link
Copy Markdown
Collaborator

All the specific code feedback has been addressed, sure. But im still unsure what causes this full_name field to be missing in the first place. Without understanding that, its very hard to evaluate whether this is the correct solution to the issue.

I would like to see more information posted in the underlying issue that provides a root cause analysis for the issue before this pr can move forward

@MoralCode MoralCode added more info needed Issues that need additional context and information to be resolved. waiting This change is waiting for some other changes to land first labels Feb 6, 2026
@MoralCode MoralCode marked this pull request as draft February 6, 2026 18:35
@HimasreeKolathur24
Copy link
Copy Markdown
Contributor Author

@MoralCode,
I’ve added some context to the issue about how the redirect URL is currently being used and my understanding of when this occurs. Thanks for pointing this out.

@MoralCode
Copy link
Copy Markdown
Collaborator

I did some digging into this and came up with a different PR to help confirm my theory for why this is happening.

#3670

Can you take a look and close this PR if you think that is a better solution?

@MoralCode
Copy link
Copy Markdown
Collaborator

This is currently waiting on a re-visit after evaluating the logs of this failure after the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

more info needed Issues that need additional context and information to be resolved. waiting This change is waiting for some other changes to land first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KeyError: full_name in github repo move task

2 participants