Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No valid Github API keys fix #2404

Merged
merged 3 commits into from
May 11, 2023
Merged

No valid Github API keys fix #2404

merged 3 commits into from
May 11, 2023

Conversation

ABrain7710
Copy link
Contributor

Description

  • I came across these issues when I saw the frontend flash that a repo was added, but it actually wasn't. The issue was that I had no valid Github API keys. To fix this issue, I made the Github API key handler raise a NoValidKeysError when there are no valid github api keys. This improves the case when all the Github API keys are expired or invalid, because it now raises an exception rather than just silently not working.
  • Now in the add repo and add org code it catches NoValidKeysError and returns False so the frontend shows that the repos weren't added
  • I also came across an issue in the view code, where it wasn't unpacking a tuple before checking if the repo or org was added. This issue is why we were always see it flash that the repos were added even it they really weren't

Notes for Reviewers
There are two imports of the NoValidKeysError in augur_operations.py that are each inside of a function, this is because adding the import at the module level, creates a circular dependency (it has proved difficult to not get circular dependencies in the model files so we have been doing imports in the functions most of the time)

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Andrew Brain <andrewbrain2019@gmail.com>
Signed-off-by: Andrew Brain <andrewbrain2019@gmail.com>
Signed-off-by: Andrew Brain <andrewbrain2019@gmail.com>
Copy link
Contributor

@IsaacMilarky IsaacMilarky left a comment

Choose a reason for hiding this comment

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

LGTM

@IsaacMilarky IsaacMilarky merged commit 96d8712 into dev May 11, 2023
@ABrain7710 ABrain7710 deleted the invalid-keys-fix branch May 18, 2023 23:58
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.

2 participants