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.

Sign up for GitHub

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

Jump to bottom

Changes working towards quantifying repos by their metrics to not overwhelm collection #2227

Merged
merged 18 commits into from Mar 9, 2023
Merged

Changes working towards quantifying repos by their metrics to not overwhelm collection #2227

merged 18 commits into from Mar 9, 2023

Conversation

IsaacMilarky
Copy link
Contributor

@IsaacMilarky IsaacMilarky commented Mar 9, 2023 •
edited

Description

  • Work towards being able to schedule a predefined load of collection by repo sizes rather than raw repo amount
  • Add functions to see how large a repo is respective of its pull_requests and issues
  • Split start_tasks.py into two to make it less overwhelming
  • Split facade collection into two hooks. One to clone/update, one to collect data. This is done so that in the future similar metrics to the pr/issue metrics to see how large a repo is respective of its commits
  • Fix issues with logic when handling dependency errors
  • Add new status for facade when it is cloning and updating called 'Initializing'
  • Change celery settings to work with newly split start_tasks.py
  • Fix issue with sqlalchemy where 'one' was used instead of 'first'.

Signed commits

  • Yes, I signed my commits.

@IsaacMilarky
create new file for utils used by start_tasks.py
5d19dbd 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
missing import
eabad4c 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
create metrics for repo
1b287dd 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
missing import
5d2d70a 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
missing import
b638cb7 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
Merge branch 'dev' into weighted-user-collection-metrics
fa3477f
@IsaacMilarky
Merge branch 'dev' into weighted-user-collection-metrics
aaa5c3f
@IsaacMilarky
split up facade collection hooks so that cloning is seperate. This is…
ced4c12 
… moving towards scheduling repos based on commit count which can't be done until the repos have been cloned

Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
first instead of one
329a344 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
sqlalchemy logic
0df9021 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
syntax
d070877 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
update kill command
5b6c411 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
bizarre celery error
e0b3df6 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
syntax
2a54777 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
Add util tasks to scheduling worker
dae0371 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky
fix never_collected check
5764499 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@IsaacMilarky IsaacMilarky added the add-feature Adds new features label Mar 9, 2023
@IsaacMilarky
schedule analysis before clone/update
cf5887c 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
sgoggins
sgoggins previously approved these changes Mar 9, 2023
View reviewed changes
Copy link
Member

@sgoggins sgoggins left a comment

Choose a reason for hiding this comment

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

I approve as far as I can tell, and some of he changes are significant enough that I want @ABrain7710 to review before merging.

augur/tasks/git/facade_tasks.py
@@ -457,43 +496,13 @@ def facade_phase(repo_git):

# Figure out what we need to do
limited_run = session.limited_run
delete_marked_repos = session.delete_marked_repos
Copy link
Member

Choose a reason for hiding this comment

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

I think we are getting rid of many of the confusing different settings. yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes some have been removed but not all. Delete_marked_repos particularly should probably be removed because we never remove repos.

augur/tasks/github/facade_github/tasks.py

except NoResultFound as e:
session.logger.debug(f"Failed local login lookup with error: {e}")
if not contributors_with_matching_name:
Copy link
Member

Choose a reason for hiding this comment

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

Will taking this out of a try except cause the collection to fail in the case that a person is not identified?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, because it uses first which will return None if it doesn't exist. Where one expects a row to exist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it will not.

augur/tasks/start_tasks.py
@@ -27,161 +27,17 @@
from augur.tasks.db.refresh_materialized_views import *
# from augur.tasks.data_analysis import *
Copy link
Member

Choose a reason for hiding this comment

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

I definitely want @ABrain7710 to review this file

@IsaacMilarky
Comments
47b18c0 
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@ABrain7710 ABrain7710 merged commit 1ea59ce into dev Mar 9, 2023
@IsaacMilarky IsaacMilarky deleted the weighted-user-collection-metrics branch March 9, 2023 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ABrain7710 ABrain7710 ABrain7710 approved these changes

@sgoggins sgoggins sgoggins left review comments

Assignees
No one assigned
Labels
add-feature Adds new features
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@IsaacMilarky @sgoggins @ABrain7710