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

Fixing Schema Script and Addressing GH API Timeout. #1855

Merged
merged 9 commits into from
Jun 10, 2022
Merged

Conversation

sgoggins
Copy link
Member

@sgoggins sgoggins commented Jun 8, 2022

Pull Request Template

@sgoggins sgoggins added server Related to the Augur server workers Related to data workers database Related to Augur's unifed data model python Pull requests that update Python code labels Jun 8, 2022
IsaacMilarky
IsaacMilarky previously approved these changes Jun 9, 2022
Copy link
Contributor

@ABrain7710 ABrain7710 left a comment

Choose a reason for hiding this comment

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

The changes in worker_git_integration.py will not fix anything related to handing out new keys to workers. It will simply not make the first key used by a worker random, whereas before the changes the first key used was the key in the config so we don't use up all stored keys first. I would not recommend doing this, because we should use the key defined in the config first.

If there is an issue with handing out new keys to workers, the update_gh_rate_limit method in worker_git_integration.py would need to be looked at.

@sgoggins
Copy link
Member Author

sgoggins commented Jun 9, 2022

@ABrain7710 : The key in the config is no greater or lesser than the other keys. The issue right now is that in practice it is not rotating through the available keys.

@ABrain7710
Copy link
Contributor

I understand they have the same number of requests, but we should use the key in the config first, because more people are likely to be using the stored keys at the same time, so we don't want to run those out first, we would rather run the one in the config first since it isn't shared.

@sgoggins
Copy link
Member Author

sgoggins commented Jun 9, 2022

@ABrain7710 : If there's only the one in the config, its going to generate a random number of 1, and only hit that key. Everything still works if there's only the API key in the config. In practice the ones in the table are isolated to the organization running the instance. So all the keys have the same "value".

@sgoggins
Copy link
Member Author

sgoggins commented Jun 9, 2022

@ABrain7710 : Right now, its simply not getting to all the keys before it goes kaput. In practice.

Signed-off-by: Sean Goggins <outdoors@acm.org>
@sgoggins sgoggins merged commit 2707616 into main Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Related to Augur's unifed data model python Pull requests that update Python code server Related to the Augur server workers Related to data workers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants