From a9b4cf84c2026c68993f49c0976d3e16afd4cbfb Mon Sep 17 00:00:00 2001 From: joseph-sentry Date: Thu, 8 Aug 2024 17:05:45 -0400 Subject: [PATCH 1/4] feat: notify_error default=True for repos created after certain date Make config option codecov.notify.notify_error default to True for repos created after a certain date. We don't want to change the behaviour for existing repos but we want this to be default behaviour for repos going forward. --- shared/config/__init__.py | 9 ++++ .../migrations/0057_repository_created_at.py | 18 ++++++++ shared/django_apps/core/models.py | 1 + shared/yaml/user_yaml.py | 45 +++++++++++++------ tests/unit/yaml/test_user_yaml.py | 28 +++++++++--- 5 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 shared/django_apps/core/migrations/0057_repository_created_at.py diff --git a/shared/config/__init__.py b/shared/config/__init__.py index ad70ee4f..da8a85f0 100644 --- a/shared/config/__init__.py +++ b/shared/config/__init__.py @@ -68,6 +68,15 @@ class MissingConfigException(Exception): }, } +NOTIFY_ERROR_TIME_START = datetime.fromisoformat("2024-08-09 00:00:00.000+00:00") + + +def add_notify_error_to_config(config: dict[str, Any]): + codecov: dict[str, Any] = config.setdefault("codecov", {}) + notify: dict[str, Any] = codecov.setdefault("notify", {}) + notify.setdefault("notify_error", True) + + default_config = { "services": { "minio": { diff --git a/shared/django_apps/core/migrations/0057_repository_created_at.py b/shared/django_apps/core/migrations/0057_repository_created_at.py new file mode 100644 index 00000000..f00ef39d --- /dev/null +++ b/shared/django_apps/core/migrations/0057_repository_created_at.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.13 on 2024-08-08 19:49 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0056_branch_name_trgm_idx"), + ] + + operations = [ + migrations.AddField( + model_name="repository", + name="created_at", + field=models.DateTimeField(auto_now_add=True, null=True), + ), + ] diff --git a/shared/django_apps/core/models.py b/shared/django_apps/core/models.py index 4ac11021..b0eefc1a 100644 --- a/shared/django_apps/core/models.py +++ b/shared/django_apps/core/models.py @@ -140,6 +140,7 @@ class Languages(models.TextChoices): bundle_analysis_enabled = models.BooleanField(default=False, null=True) coverage_enabled = models.BooleanField(default=False, null=True) test_analytics_enabled = models.BooleanField(default=False, null=True) + created_at = models.DateTimeField(null=True, auto_now_add=True) # tracks field changes being saved tracker = FieldTracker() diff --git a/shared/yaml/user_yaml.py b/shared/yaml/user_yaml.py index 43ac4262..d8acaaf8 100644 --- a/shared/yaml/user_yaml.py +++ b/shared/yaml/user_yaml.py @@ -6,8 +6,10 @@ from shared.components import Component from shared.config import ( + NOTIFY_ERROR_TIME_START, PATCH_CENTRIC_DEFAULT_CONFIG, PATCH_CENTRIC_DEFAULT_TIME_START, + add_notify_error_to_config, get_config, ) @@ -21,6 +23,11 @@ class OwnerContext(object): owner_plan: Optional[str] = None +@dataclass +class RepoContext(object): + repo_creation_date: datetime | None = None + + class UserYaml(object): def __init__(self, inner_dict): self.inner_dict = inner_dict @@ -119,11 +126,12 @@ def __str__(self): def get_final_yaml( cls, *, - owner_yaml: Optional[dict[str, Any]] = None, - repo_yaml: Optional[dict[str, Any]] = None, - commit_yaml: Optional[dict[str, Any]] = None, - ownerid: int = None, - owner_context: Optional[OwnerContext] = None, + owner_yaml: dict[str, Any] | None = None, + repo_yaml: dict[str, Any] | None = None, + commit_yaml: dict[str, Any] | None = None, + ownerid: int | None = None, + owner_context: OwnerContext | None = None, + repo_context: RepoContext | None = None, ): """Given a owner yaml, repo yaml and user yaml, determines what yaml we need to use @@ -159,10 +167,11 @@ def get_final_yaml( additional_yaml = _get_possible_additional_user_yaml(ownerid) resulting_yaml = merge_yamls(resulting_yaml, additional_yaml) - if owner_context and owner_context.owner_onboarding_date: - resulting_yaml = _fix_yaml_defaults_based_on_owner_onboarding_date( - resulting_yaml, owner_context.owner_onboarding_date - ) + resulting_yaml = _fix_yaml_defaults_based_on_time( + resulting_yaml, + getattr(owner_context, "owner_onboarding_date", None), + getattr(repo_context, "repo_creation_date", None), + ) if owner_yaml is not None: resulting_yaml = merge_yamls(resulting_yaml, owner_yaml) @@ -185,8 +194,10 @@ def _get_possible_additional_user_yaml(ownerid): return {} -def _fix_yaml_defaults_based_on_owner_onboarding_date( - current_yaml: dict, owner_onboarding_date: datetime +def _fix_yaml_defaults_based_on_time( + current_yaml: dict, + owner_onboarding_date: datetime | None, + repo_creation_date: datetime | None, ) -> dict: """Changes the site defaults based on the owner_onboarding_date. Owners onboarded (Owner.created_at) after PATCH_CENTRIC_DEFAULT_TIME_START use the @@ -195,9 +206,15 @@ def _fix_yaml_defaults_based_on_owner_onboarding_date( Owners can still override the defaults through owner_yaml, repo_yaml and commit_yaml. """ res = deepcopy(current_yaml) - if owner_onboarding_date > PATCH_CENTRIC_DEFAULT_TIME_START: - adding = deepcopy(PATCH_CENTRIC_DEFAULT_CONFIG) - res.update(adding) + if owner_onboarding_date: + if owner_onboarding_date > PATCH_CENTRIC_DEFAULT_TIME_START: + adding = deepcopy(PATCH_CENTRIC_DEFAULT_CONFIG) + res.update(adding) + + if repo_creation_date: + if repo_creation_date > NOTIFY_ERROR_TIME_START: + add_notify_error_to_config(res) + return res diff --git a/tests/unit/yaml/test_user_yaml.py b/tests/unit/yaml/test_user_yaml.py index 3a3140ee..0246a4a8 100644 --- a/tests/unit/yaml/test_user_yaml.py +++ b/tests/unit/yaml/test_user_yaml.py @@ -1,4 +1,5 @@ import datetime +from copy import deepcopy from freezegun import freeze_time @@ -11,7 +12,7 @@ from shared.yaml import UserYaml, merge_yamls from shared.yaml.user_yaml import ( OwnerContext, - _fix_yaml_defaults_based_on_owner_onboarding_date, + _fix_yaml_defaults_based_on_time, _get_possible_additional_user_yaml, ) @@ -348,21 +349,38 @@ def test_get_final_yaml_nothing(self, mock_configuration): def test_default_yaml_behavior_change(self): current_yaml = LEGACY_DEFAULT_SITE_CONFIG day_timedelta = datetime.timedelta(days=1) + year_timedelta = datetime.timedelta(days=365) patch_centric_expected_onboarding_date = ( datetime.datetime.now(datetime.timezone.utc) + day_timedelta ) no_change_expected_onboarding_date = ( datetime.datetime.now(datetime.timezone.utc) - day_timedelta ) - no_change = _fix_yaml_defaults_based_on_owner_onboarding_date( - current_yaml, no_change_expected_onboarding_date + notify_error_expected_onboarding_date = ( + datetime.datetime.now(datetime.timezone.utc) + year_timedelta + ) + no_change = _fix_yaml_defaults_based_on_time( + current_yaml, + no_change_expected_onboarding_date, + no_change_expected_onboarding_date, ) assert no_change == current_yaml - patch_centric = _fix_yaml_defaults_based_on_owner_onboarding_date( - current_yaml, patch_centric_expected_onboarding_date + patch_centric = _fix_yaml_defaults_based_on_time( + current_yaml, + patch_centric_expected_onboarding_date, + no_change_expected_onboarding_date, ) assert patch_centric != current_yaml assert patch_centric == PATCH_CENTRIC_DEFAULT_CONFIG + patch_centric = _fix_yaml_defaults_based_on_time( + current_yaml, + patch_centric_expected_onboarding_date, + notify_error_expected_onboarding_date, + ) + assert patch_centric != current_yaml + notify_error_config = deepcopy(PATCH_CENTRIC_DEFAULT_CONFIG) + notify_error_config["codecov"]["notify"]["notify_error"] = True + assert patch_centric == notify_error_config def test_get_final_yaml_default_based_on_owner_context(self): day_timedelta = datetime.timedelta(days=1) From 4e411fe8d1c3404401ee6657fea5626ce210338b Mon Sep 17 00:00:00 2001 From: joseph-sentry Date: Thu, 8 Aug 2024 17:18:05 -0400 Subject: [PATCH 2/4] make lint and push date back --- shared/config/__init__.py | 2 +- .../core/migrations/0057_repository_created_at.py | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/shared/config/__init__.py b/shared/config/__init__.py index da8a85f0..5cfb8aa1 100644 --- a/shared/config/__init__.py +++ b/shared/config/__init__.py @@ -68,7 +68,7 @@ class MissingConfigException(Exception): }, } -NOTIFY_ERROR_TIME_START = datetime.fromisoformat("2024-08-09 00:00:00.000+00:00") +NOTIFY_ERROR_TIME_START = datetime.fromisoformat("2024-09-02 00:00:00.000+00:00") def add_notify_error_to_config(config: dict[str, Any]): diff --git a/shared/django_apps/core/migrations/0057_repository_created_at.py b/shared/django_apps/core/migrations/0057_repository_created_at.py index f00ef39d..cd26ce04 100644 --- a/shared/django_apps/core/migrations/0057_repository_created_at.py +++ b/shared/django_apps/core/migrations/0057_repository_created_at.py @@ -2,9 +2,18 @@ from django.db import migrations, models +""" +BEGIN; +-- +-- Add field created_at to repository +-- +ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone DEFAULT '2024-08-08T21:17:39.913910+00:00'::timestamptz NULL; +ALTER TABLE "repos" ALTER COLUMN "created_at" DROP DEFAULT; +COMMIT; +""" -class Migration(migrations.Migration): +class Migration(migrations.Migration): dependencies = [ ("core", "0056_branch_name_trgm_idx"), ] From 4ff7be810ff0e717e9ff190c05cfe0aa06791a9e Mon Sep 17 00:00:00 2001 From: joseph-sentry Date: Wed, 14 Aug 2024 11:42:23 -0400 Subject: [PATCH 3/4] fix: replace created_at default with null --- .../core/migrations/0057_repository_created_at.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/shared/django_apps/core/migrations/0057_repository_created_at.py b/shared/django_apps/core/migrations/0057_repository_created_at.py index cd26ce04..c32f10dd 100644 --- a/shared/django_apps/core/migrations/0057_repository_created_at.py +++ b/shared/django_apps/core/migrations/0057_repository_created_at.py @@ -1,14 +1,13 @@ # Generated by Django 4.2.13 on 2024-08-08 19:49 -from django.db import migrations, models +from django.db import migrations """ BEGIN; -- --- Add field created_at to repository +-- Raw SQL operation -- -ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone DEFAULT '2024-08-08T21:17:39.913910+00:00'::timestamptz NULL; -ALTER TABLE "repos" ALTER COLUMN "created_at" DROP DEFAULT; +ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone DEFAULT null NULL; COMMIT; """ @@ -19,9 +18,7 @@ class Migration(migrations.Migration): ] operations = [ - migrations.AddField( - model_name="repository", - name="created_at", - field=models.DateTimeField(auto_now_add=True, null=True), + migrations.RunSQL( + 'ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone DEFAULT null NULL;' ), ] From 96f270e6be7756c6a8a8817b7c9385c168ec2403 Mon Sep 17 00:00:00 2001 From: joseph-sentry Date: Wed, 28 Aug 2024 12:43:29 -0400 Subject: [PATCH 4/4] fix: address feedback --- .../django_apps/core/migrations/0057_repository_created_at.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared/django_apps/core/migrations/0057_repository_created_at.py b/shared/django_apps/core/migrations/0057_repository_created_at.py index c32f10dd..407610fa 100644 --- a/shared/django_apps/core/migrations/0057_repository_created_at.py +++ b/shared/django_apps/core/migrations/0057_repository_created_at.py @@ -7,7 +7,7 @@ -- -- Raw SQL operation -- -ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone DEFAULT null NULL; +ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone NULL; COMMIT; """ @@ -19,6 +19,6 @@ class Migration(migrations.Migration): operations = [ migrations.RunSQL( - 'ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone DEFAULT null NULL;' + 'ALTER TABLE "repos" ADD COLUMN "created_at" timestamp with time zone NULL;' ), ]