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

Move Facade Affiliation Refresh and Rebuild to Happen After Materialized Views Refresh #2820

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

IsaacMilarky
Copy link
Contributor

Description

  • Move the Facade affiliation refresh dm_table logic into the same place that we refresh the materialized views as per @sgoggins suggestion
  • Solves problem with logic being executed too frequently.

Notes for Reviewers
Hasn't been tested quite yet. I will make it a real PR when I have tested it.

Although feel free to test it yourself.

Signed commits

  • Yes, I signed my commits.

@@ -163,6 +166,36 @@ def refresh_materialized_views(self):
logger.info(f"error is {e}")
pass

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
W0107: Unnecessary pass statement (unnecessary-pass)

@@ -4,6 +4,9 @@

from augur.tasks.init.celery_app import celery_app as celery
from augur.application.db.lib import execute_sql
from augur.tasks.git.util.facade_worker.facade_worker.config import FacadeHelper
from augur.tasks.git.util.facade_worker.facade_worker.rebuildcache import invalidate_caches, rebuild_unknown_affiliation_and_web_caches


@celery.task(bind=True)
def refresh_materialized_views(self):

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
R0912: Too many branches (19/12) (too-many-branches)

@@ -4,6 +4,9 @@

from augur.tasks.init.celery_app import celery_app as celery
from augur.application.db.lib import execute_sql
from augur.tasks.git.util.facade_worker.facade_worker.config import FacadeHelper
from augur.tasks.git.util.facade_worker.facade_worker.rebuildcache import invalidate_caches, rebuild_unknown_affiliation_and_web_caches


@celery.task(bind=True)
def refresh_materialized_views(self):

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
R0915: Too many statements (96/50) (too-many-statements)

@@ -10,8 +10,6 @@
from augur.tasks.git.util.facade_worker.facade_worker.utilitymethods import get_absolute_repo_path, get_parent_commits_set, get_existing_commits_set
from augur.tasks.git.util.facade_worker.facade_worker.analyzecommit import analyze_commit
from augur.tasks.git.util.facade_worker.facade_worker.utilitymethods import get_repo_commit_count, update_facade_scheduling_fields, get_facade_weight_with_commit_count
from augur.tasks.git.util.facade_worker.facade_worker.rebuildcache import fill_empty_affiliations, invalidate_caches, nuke_affiliations, rebuild_unknown_affiliation_and_web_caches


from augur.tasks.github.facade_github.tasks import *

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
W0401: Wildcard import augur.tasks.github.facade_github.tasks (wildcard-import)

@@ -140,8 +140,6 @@ def non_repo_domain_tasks(self):

enabled_tasks = []

enabled_tasks.extend(generate_non_repo_domain_facade_tasks(logger))

if machine_learning_phase.__name__ in enabled_phase_names:

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
E0601: Using variable 'machine_learning_phase' before assignment (used-before-assignment)

@@ -140,8 +140,6 @@ def non_repo_domain_tasks(self):

enabled_tasks = []

enabled_tasks.extend(generate_non_repo_domain_facade_tasks(logger))

if machine_learning_phase.__name__ in enabled_phase_names:
#enabled_tasks.extend(machine_learning_phase())
from augur.tasks.data_analysis.contributor_breadth_worker.contributor_breadth_worker import contributor_breadth_model

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
C0415: Import outside toplevel (augur.tasks.data_analysis.contributor_breadth_worker.contributor_breadth_worker.contributor_breadth_model) (import-outside-toplevel)

@sgoggins
Copy link
Member

@IsaacMilarky : Is this ready?

@IsaacMilarky
Copy link
Contributor Author

@IsaacMilarky : Is this ready?

Yes sorry I think I forgot to make this a real PR.

@IsaacMilarky IsaacMilarky marked this pull request as ready for review June 24, 2024 21:30
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.

@IsaacMilarky : Can you explain this one a little bit more.

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.

LGTM

@@ -10,8 +10,6 @@
from augur.tasks.git.util.facade_worker.facade_worker.utilitymethods import get_absolute_repo_path, get_parent_commits_set, get_existing_commits_set
from augur.tasks.git.util.facade_worker.facade_worker.analyzecommit import analyze_commit
from augur.tasks.git.util.facade_worker.facade_worker.utilitymethods import get_repo_commit_count, update_facade_scheduling_fields, get_facade_weight_with_commit_count
from augur.tasks.git.util.facade_worker.facade_worker.rebuildcache import fill_empty_affiliations, invalidate_caches, nuke_affiliations, rebuild_unknown_affiliation_and_web_caches
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 want fill_empty_affiliations to still work.

@sgoggins sgoggins merged commit 0db3ba3 into dev Jun 25, 2024
9 checks passed
@IsaacMilarky IsaacMilarky deleted the facade-non-domain-patch branch October 15, 2024 23:25
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