Worker: Migrate to Plan / Tier Tables #1007

ajay-sentry · 2025-01-16T00:14:06Z

This PR relies on this version of shared: Shared: Migrate to Plan / Tier Tables shared#479.

The main objective of this PR is to migrate all Backend Service logic to use the new plan and tiers tables instead of the Plan constants. You can read more about milestone 3 here.

A core change introduced in this PR is the creation of mocks for plans and tiers, which are used in most test setup functions. For tests requiring usage in all nested functions, we use a fixture with autouse enabled. Additionally, we ensure django_db decorators are included to allow access to the mock database.

The rest of the changes should be fairly straightforward.

Closes codecov/engineering-team#3253

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

codecov-staging · 2025-01-16T00:20:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

codecov-qa · 2025-01-16T00:20:47Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
1793	2	1791	9

View the top 2 failed tests by shortest run time

services/notification/notifiers/tests/unit/test_comment.py::TestCommentNotifier::test_send_actual_notification_spammy

Stack Traces | 0.027s run time

self = <test_comment.TestCommentNotifier object at 0x7f64d85532a0>

    @pytest.fixture(autouse=True)
    def setup(self):
>       mock_all_plans_and_tiers()

.../tests/unit/test_comment.py:445: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/helpers.py:7: in mock_all_plans_and_tiers
    trial_tier = TierFactory(tier_name=TierName.TRIAL.value)
.../local/lib/python3.13............/site-packages/factory/base.py:40: in __call__
    return cls.create(**kwargs)
.../local/lib/python3.13............/site-packages/factory/base.py:528: in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
.../local/lib/python3.13....../site-packages/factory/django.py:117: in _generate
    return super()._generate(strategy, params)
.../local/lib/python3.13............/site-packages/factory/base.py:465: in _generate
    return step.build()
.../local/lib/python3.13.../site-packages/factory/builder.py:262: in build
    instance = self.factory_meta.instantiate(
.../local/lib/python3.13............/site-packages/factory/base.py:317: in instantiate
    return self.factory._create(model, *args, **kwargs)
.../local/lib/python3.13....../site-packages/factory/django.py:166: in _create
    return manager.create(*args, **kwargs)
.../local/lib/python3.13.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.13.../db/models/query.py:658: in create
    obj.save(force_insert=True, using=self.db)
.../local/lib/python3.13.../db/models/base.py:814: in save
    self.save_base(
.../local/lib/python3.13.../db/models/base.py:877: in save_base
    updated = self._save_table(
.../local/lib/python3.13.../db/models/base.py:1020: in _save_table
    results = self._do_insert(
.../local/lib/python3.13.../db/models/base.py:1061: in _do_insert
    return manager._insert(
.../local/lib/python3.13.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.13.../db/models/query.py:1805: in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
.../local/lib/python3.13.../models/sql/compiler.py:1820: in execute_sql
    with self.connection.cursor() as cursor:
.../local/lib/python3.13.../django/utils/asyncio.py:26: in inner
    return func(*args, **kwargs)
.../local/lib/python3.13.../backends/base/base.py:330: in cursor
    return self._cursor()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <DatabaseWrapper vendor='postgresql' alias='default'>, name = None

    def _cursor(self, name=None):
        self.close_if_health_check_failed()
>       self.ensure_connection()
E       RuntimeError: Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" fixtures to enable it.

.../local/lib/python3.13.../backends/base/base.py:306: RuntimeError

services/notification/notifiers/tests/unit/test_comment.py::TestCommentNotifier::test_build_message_default_layout

Stack Traces | 0.034s run time

self = <test_comment.TestCommentNotifier object at 0x7f64d8550500>

    @pytest.fixture(autouse=True)
    def setup(self):
>       mock_all_plans_and_tiers()

.../tests/unit/test_comment.py:445: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/helpers.py:7: in mock_all_plans_and_tiers
    trial_tier = TierFactory(tier_name=TierName.TRIAL.value)
.../local/lib/python3.13............/site-packages/factory/base.py:40: in __call__
    return cls.create(**kwargs)
.../local/lib/python3.13............/site-packages/factory/base.py:528: in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
.../local/lib/python3.13....../site-packages/factory/django.py:117: in _generate
    return super()._generate(strategy, params)
.../local/lib/python3.13............/site-packages/factory/base.py:465: in _generate
    return step.build()
.../local/lib/python3.13.../site-packages/factory/builder.py:262: in build
    instance = self.factory_meta.instantiate(
.../local/lib/python3.13............/site-packages/factory/base.py:317: in instantiate
    return self.factory._create(model, *args, **kwargs)
.../local/lib/python3.13....../site-packages/factory/django.py:166: in _create
    return manager.create(*args, **kwargs)
.../local/lib/python3.13.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.13.../db/models/query.py:658: in create
    obj.save(force_insert=True, using=self.db)
.../local/lib/python3.13.../db/models/base.py:814: in save
    self.save_base(
.../local/lib/python3.13.../db/models/base.py:877: in save_base
    updated = self._save_table(
.../local/lib/python3.13.../db/models/base.py:1020: in _save_table
    results = self._do_insert(
.../local/lib/python3.13.../db/models/base.py:1061: in _do_insert
    return manager._insert(
.../local/lib/python3.13.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.13.../db/models/query.py:1805: in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
.../local/lib/python3.13.../models/sql/compiler.py:1820: in execute_sql
    with self.connection.cursor() as cursor:
.../local/lib/python3.13.../django/utils/asyncio.py:26: in inner
    return func(*args, **kwargs)
.../local/lib/python3.13.../backends/base/base.py:330: in cursor
    return self._cursor()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <DatabaseWrapper vendor='postgresql' alias='default'>, name = None

    def _cursor(self, name=None):
        self.close_if_health_check_failed()
>       self.ensure_connection()
E       RuntimeError: Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" fixtures to enable it.

.../local/lib/python3.13.../backends/base/base.py:306: RuntimeError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

codecov-public-qa · 2025-01-16T00:20:56Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
1793	2	1791	9

View the top 2 failed tests by shortest run time

services/notification/notifiers/tests/unit/test_comment.py::TestCommentNotifier::test_send_actual_notification_spammy

Stack Traces | 0.027s run time

No failure message available

services/notification/notifiers/tests/unit/test_comment.py::TestCommentNotifier::test_build_message_default_layout

Stack Traces | 0.034s run time

No failure message available

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

github-actions · 2025-01-16T00:21:06Z

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

codecov · 2025-01-16T00:34:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@5e92d54). Learn more about missing BASE report.
Report is 11 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1007   +/-   ##
=======================================
  Coverage        ?   97.52%           
=======================================
  Files           ?      462           
  Lines           ?    37899           
  Branches        ?        0           
=======================================
  Hits            ?    36960           
  Misses          ?      939           
  Partials        ?        0

Flag	Coverage Δ
integration	`42.89% <90.34%> (?)`
unit	`90.17% <99.06%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

⚠️ Impact Analysis from Codecov is deprecated and will be sunset on Jan 31 2025. See more

RulaKhaled · 2025-01-20T08:58:44Z

database/models/core.py

-        types.String(50), nullable=False, default=PlanName.BASIC_PLAN_NAME.value
+        types.String(50),
+        nullable=False,
+        default=PlanName.BASIC_PLAN_NAME.value,  # TODO: UPDATE WITH NEW FREE PLAN NAME


Good catch!

requirements.txt

github-actions · 2025-01-23T12:25:40Z

This PR includes changes to shared. Please review them here: codecov/shared@d1762e3...906e7bc

ajay-sentry · 2025-01-23T18:04:12Z

services/tests/test_decoration.py

@@ -491,12 +500,14 @@ def test_get_decoration_type_not_pr_plan(self, dbsession, mocker, enriched_pull)
        assert decoration_details.reason == "Org not on PR plan"
        assert decoration_details.should_attempt_author_auto_activation is False

+    @pytest.mark.django_db
+    # what is a users plan?


users plan is the one from GHM, which we actually will need to add to the plan table in DB as well now that I think about it

oohhh cool cool

Swatinem

Thanks for cleaning up all that print noise in test code (and in production code, lol). If its not too much effort, can you extract those changes as a separate PR?

Otherwise, my comments related to having this mock data around globally instead of pulling it in explicitly. As well as consuming these objects from the DB rather than by mocking function calls would be nice.
Mixing django/sqlalchemy is also something to be cautious about, as I believe it could cause massive problems.

Swatinem · 2025-01-24T11:59:00Z

services/notification/tests/unit/test_notification_service.py

+    @patch("services.notification.Plan.objects.get")
    def test_should_use_checks_notifier_deprecated_flow(
-        self, repo_data, outcome, dbsession
+        self, plan_objects_get, repo_data, outcome, dbsession
    ):
+        plan = PlanFactory.create()
+        plan_objects_get.return_value = plan


I have some questions, and see some problems with this:

first of all, you are creating the entity based on the Factory, so you might as well instert this into the database instead of using a mock for it.

but judging by the mock, this looks like you are using django .objects.get somewhere internally. This does not mix well with sqlalchemy (judging by both the dbsession fixture, and the .create test factory.

I believe that also in production, we should not mix these two ORMs, as that can potentially cause deadlocks.

tasks/upload.py

Swatinem · 2025-01-24T12:03:58Z

tests/helpers.py

+from shared.plan.constants import PlanName, PlanPrice, TierName
+
+
+def mock_all_plans_and_tiers():


you are calling this explicitly in a ton of tests.

How is this being handled in production? Were these things created manually, or using a data migration?

Maybe it makes sense to introduce these kinds of "constants" using a data migration in the first place.
Otherwise, is it possible to auto-load these fixtures in all the tests that are using the django DB somehow?
Its a bit annoying that you have to call this manually in all those places. Better to just "have this data around" within your tests.

github-actions · 2025-01-27T18:38:28Z

This PR includes changes to shared. Please review them here: codecov/shared@e854f50...7804571

github-actions · 2025-01-29T23:18:55Z

This PR includes changes to shared. Please review them here: codecov/shared@ce6e1e9...74c0888

worker plan updates to use new plan model

1be033f

ajay-sentry added 3 commits January 16, 2025 14:07

Merge branch 'main' into Ajay/milestone-3-migration

a272a6a

Merge branch 'main' into Ajay/milestone-3-migration

ccc2534

81 failing tests, realized should start with shared

7216f13

RulaKhaled reviewed Jan 20, 2025

View reviewed changes

requirements.txt Outdated Show resolved Hide resolved

ajay-sentry changed the title ~~[WIP] Milestone 3 Plan Migration~~ Worker: Migrate to Plan / Tier Tables Jan 23, 2025

RulaKhaled added 2 commits January 23, 2025 10:31

Merge branch 'main' into Ajay/milestone-3-migration

b22b6e0

resolve all unit tests

aaffe2d

RulaKhaled and others added 2 commits January 23, 2025 15:28

resolve all unit tests

fe2f21e

get rid of prints

0a938c6

ajay-sentry commented Jan 23, 2025

View reviewed changes

ajay-sentry and others added 2 commits January 23, 2025 10:17

prefetch tier when needed

9fdf81c

Merge branch 'main' into Ajay/milestone-3-migration

f1eb632

Swatinem reviewed Jan 24, 2025

View reviewed changes

ajay-sentry and others added 4 commits January 24, 2025 09:48

Merge branch 'main' into Ajay/milestone-3-migration

70f7dea

update to not use @patch(services.notification.Plan.objects.get)

cdc672e

update with more tests

481d3a9

update with shared version

da4ed81

Merge branch 'main' into Ajay/milestone-3-migration

2002db3

RulaKhaled temporarily deployed to staging January 28, 2025 10:08 — with GitHub Actions Inactive

Swatinem approved these changes Jan 28, 2025

View reviewed changes

ajay-sentry mentioned this pull request Jan 29, 2025

Shared: Migrate to Plan / Tier Tables codecov/shared#479

Merged

ajay-sentry added 2 commits January 29, 2025 15:17

Merge branch 'main' into Ajay/milestone-3-migration

1c43bb0

testing stage

801ad17

ajay-sentry added 2 commits January 29, 2025 15:36

fix tests

111e942

fix these other tests

0aa6e04

ajay-sentry temporarily deployed to staging January 30, 2025 00:34 — with GitHub Actions Inactive

update with new shared version

759bc66

RulaKhaled added this pull request to the merge queue Jan 30, 2025

Merged via the queue into main with commit 5ee50a8 Jan 30, 2025
26 of 27 checks passed

RulaKhaled deleted the Ajay/milestone-3-migration branch January 30, 2025 17:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Worker: Migrate to Plan / Tier Tables #1007

Worker: Migrate to Plan / Tier Tables #1007

ajay-sentry commented Jan 16, 2025 •

edited by RulaKhaled

Loading

codecov-staging bot commented Jan 16, 2025 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented Jan 16, 2025 •

edited

Loading

codecov-public-qa bot commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading

codecov bot commented Jan 16, 2025 •

edited

Loading

RulaKhaled Jan 20, 2025

github-actions bot commented Jan 23, 2025

ajay-sentry Jan 23, 2025

RulaKhaled Jan 24, 2025

Swatinem left a comment

Swatinem Jan 24, 2025

Swatinem Jan 24, 2025

github-actions bot commented Jan 27, 2025

github-actions bot commented Jan 29, 2025 •

edited

Loading

		from shared.plan.constants import PlanName, PlanPrice, TierName


		def mock_all_plans_and_tiers():

Worker: Migrate to Plan / Tier Tables #1007

Worker: Migrate to Plan / Tier Tables #1007

Conversation

ajay-sentry commented Jan 16, 2025 • edited by RulaKhaled Loading

Legal Boilerplate

codecov-staging bot commented Jan 16, 2025 • edited by codecov-notifications bot Loading

Codecov Report

codecov-qa bot commented Jan 16, 2025 • edited Loading

❌ 2 Tests Failed:

codecov-public-qa bot commented Jan 16, 2025 • edited Loading

❌ 2 Tests Failed:

github-actions bot commented Jan 16, 2025 • edited Loading

codecov bot commented Jan 16, 2025 • edited Loading

Codecov Report

RulaKhaled Jan 20, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 23, 2025

ajay-sentry Jan 23, 2025

Choose a reason for hiding this comment

RulaKhaled Jan 24, 2025

Choose a reason for hiding this comment

Swatinem left a comment

Choose a reason for hiding this comment

Swatinem Jan 24, 2025

Choose a reason for hiding this comment

Swatinem Jan 24, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 27, 2025

github-actions bot commented Jan 29, 2025 • edited Loading

ajay-sentry commented Jan 16, 2025 •

edited by RulaKhaled

Loading

codecov-staging bot commented Jan 16, 2025 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented Jan 16, 2025 •

edited

Loading

codecov-public-qa bot commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading

codecov bot commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 29, 2025 •

edited

Loading