add FlareCleanupTask

[nora-codecov]

New strategy for managing flare storage

depends on codecov/shared#450

When a pull is no longer OPEN, leave the flare in order to reduce locks and wait time.
During low traffic hours (4am UTC?), run this task to clear it out of our database and Archive storage.
So we won't pay for maintaining the flare field but also won't slow down other pull actions by checking or clearing the field 🎉

[github-actions]

This PR includes changes to shared. Please review them here: codecov/shared@45252f7...9f5c4fd

[codecov]

Codecov Report

Attention: Patch coverage is 96.02273% with 7 lines in your changes missing coverage. Please review.

Project coverage is 90.63%. Comparing base (1c06547) to head (20ccfb7).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
database/models/core.py	55.55%	4 Missing ⚠️
tasks/flare_cleanup.py	93.87%	3 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (1c06547) and HEAD (20ccfb7). Click for more details.

HEAD has 3 uploads less than BASE

Flag	BASE (1c06547)	HEAD (20ccfb7)
unit	2	1
integration	2	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #947      +/-   ##
==========================================
- Coverage   97.96%   90.63%   -7.33%     
==========================================
  Files         444      446       +2     
  Lines       35879    36045     +166     
==========================================
- Hits        35148    32671    -2477     
- Misses        731     3374    +2643     

Flag	Coverage Δ
integration	?
unit	90.63% <96.02%> (+0.03%)	⬆️

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

[codecov-notifications]

Codecov Report

Attention: Patch coverage is 97.72727% with 4 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
tasks/flare_cleanup.py	93.87%	3 Missing ⚠️
database/models/core.py	88.88%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

✅ All tests successful. No failed tests were found.

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

[Swatinem]

A couple of suggestions:

• We should definitely limit / batch the query. This could yield millions of entries on the first run for all we know 🤷🏻‍♂️
• Using the mock_storage fixture, you could assert the actual side effect of files being removed from the storage backend, instead of manually mocking very specific ArchiveService methods

Otherwise this looks good, and the following are just "personal preference" suggestions:

• IMO, we should move core business logic out of the celery framework code. In this case this would mean having some free functions instead of all the logic being in FlareCleanupTask.run_cron_task. This means you don’t even need the db_session argument, as the code does not use sqlalchemy models, and it should make testing less awkward.
• I’m personally not a fan of asserting log calls, but rather just the side effects we are aiming for: files being removed from storage, fields being cleared from the DB.

[github-actions]

This PR includes changes to shared. Please review them here: codecov/shared@2674ae9...2331c4a

[codecov-qa]

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1769	1	1768	1

View the top 1 failed tests by shortest run time

tasks/tests/unit/test_flare_cleanup.py::TestFlareCleanupTask::test_successful_run

Stack Traces | 0.309s run time

self = <worker.tasks.tests.unit.test_flare_cleanup.TestFlareCleanupTask object at 0x7f4e421ca210>
transactional_db = None
mocker = <pytest_mock.plugin.MockFixture object at 0x7f4e3be52510>

    def test_successful_run(self, transactional_db, mocker):
        mock_logs = mocker.patch("logging.Logger.info")
        mock_archive_service = mocker.patch(
            "shared.django_apps.utils.model_utils.ArchiveService"
        )
        archive_value_for_flare = {"some": "data"}
        mock_archive_service.return_value.read_file.return_value = json.dumps(
            archive_value_for_flare
        )
        mock_path = ".../to/written/object"
        mock_archive_service.return_value.write_json_data_to_storage.return_value = (
            mock_path
        )
        mock_archive_service_in_task = mocker.patch(
            "tasks.flare_cleanup.ArchiveService"
        )
        mock_archive_service_in_task.return_value.delete_file.return_value = None
    
        local_value_for_flare = {"test": "test"}
        open_pull_with_local_flare = PullFactory(
            state=PullStates.OPEN.value,
            _flare=local_value_for_flare,
            repository=RepositoryFactory(),
        )
>       assert open_pull_with_local_flare.flare == local_value_for_flare
E       AssertionError: assert {} == {'test': 'test'}
E         
E         Right contains 1 more item:
E         {'test': 'test'}
E         Use -v to get more diff

.../tests/unit/test_flare_cleanup.py:42: AssertionError

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

[codecov-public-qa]

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1769	1	1768	1

View the top 1 failed tests by shortest run time

tasks/tests/unit/test_flare_cleanup.py::TestFlareCleanupTask::test_successful_run

Stack Traces | 0.309s run time

self = <worker.tasks.tests.unit.test_flare_cleanup.TestFlareCleanupTask object at 0x7f4e421ca210>
transactional_db = None
mocker = <pytest_mock.plugin.MockFixture object at 0x7f4e3be52510>

    def test_successful_run(self, transactional_db, mocker):
        mock_logs = mocker.patch("logging.Logger.info")
        mock_archive_service = mocker.patch(
            "shared.django_apps.utils.model_utils.ArchiveService"
        )
        archive_value_for_flare = {"some": "data"}
        mock_archive_service.return_value.read_file.return_value = json.dumps(
            archive_value_for_flare
        )
        mock_path = ".../to/written/object"
        mock_archive_service.return_value.write_json_data_to_storage.return_value = (
            mock_path
        )
        mock_archive_service_in_task = mocker.patch(
            "tasks.flare_cleanup.ArchiveService"
        )
        mock_archive_service_in_task.return_value.delete_file.return_value = None
    
        local_value_for_flare = {"test": "test"}
        open_pull_with_local_flare = PullFactory(
            state=PullStates.OPEN.value,
            _flare=local_value_for_flare,
            repository=RepositoryFactory(),
        )
>       assert open_pull_with_local_flare.flare == local_value_for_flare
E       AssertionError: assert {} == {'test': 'test'}
E         
E         Right contains 1 more item:
E         {'test': 'test'}
E         Use -v to get more diff

.../tests/unit/test_flare_cleanup.py:42: AssertionError

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

[nora-codecov]

Going to test manually before I add this to the nightly cron jobs

[michelletran-codecov]

Generally LGTM

[github-actions]

This PR includes changes to shared. Please review them here: codecov/shared@2674ae9...efe4835