Skip to content

Commit

Permalink
add FlareCleanupTask
Browse files Browse the repository at this point in the history
  • Loading branch information
nora-codecov committed Dec 6, 2024
1 parent 96bed1f commit 8b57acf
Show file tree
Hide file tree
Showing 5 changed files with 203 additions and 3 deletions.
8 changes: 8 additions & 0 deletions celery_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from shared.celery_config import (
BaseCeleryConfig,
brolly_stats_rollup_task_name,
flare_cleanup_task_name,
gh_app_webhook_check_task_name,
health_check_task_name,
profiling_finding_task_name,
Expand Down Expand Up @@ -94,6 +95,13 @@ def _beat_schedule():
"cron_task_generation_time_iso": BeatLazyFunc(get_utc_now_as_iso_format)
},
},
"flare_cleanup": {
"task": flare_cleanup_task_name,
"schedule": crontab(minute="0", hour="4"), # every day, 4am UTC (8pm PT)
"kwargs": {
"cron_task_generation_time_iso": BeatLazyFunc(get_utc_now_as_iso_format)
},
},
}

if get_config("setup", "find_uncollected_profilings", "enabled", default=True):
Expand Down
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
https://github.com/codecov/test-results-parser/archive/c840502d1b4dd7d05b2efc2c1328affaf2acd27c.tar.gz#egg=test-results-parser
https://github.com/codecov/shared/archive/45252f75524c38172b991a960bac964ec7b6f7d1.tar.gz#egg=shared
https://github.com/codecov/shared/archive/9f5c4fdd00fdc9b47e2aa76367dfce338725f2be.tar.gz#egg=shared
https://github.com/codecov/timestring/archive/d37ceacc5954dff3b5bd2f887936a98a668dda42.tar.gz#egg=timestring
asgiref>=3.7.2
analytics-python==1.3.0b1
Expand Down
6 changes: 4 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ sentry-sdk==2.13.0
# shared
setuptools==75.6.0
# via nodeenv
shared @ https://github.com/codecov/shared/archive/45252f75524c38172b991a960bac964ec7b6f7d1.tar.gz#egg=shared
shared @ https://github.com/codecov/shared/archive/9f5c4fdd00fdc9b47e2aa76367dfce338725f2be.tar.gz#egg=shared
# via -r requirements.in
six==1.16.0
# via
Expand Down Expand Up @@ -399,4 +399,6 @@ wrapt==1.16.0
yarl==1.9.4
# via vcrpy
zstandard==0.23.0
# via -r requirements.in
# via
# -r requirements.in
# shared
63 changes: 63 additions & 0 deletions tasks/flare_cleanup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import logging

from shared.api_archive.archive import ArchiveService
from shared.celery_config import flare_cleanup_task_name
from shared.django_apps.core.models import Pull, PullStates

from app import celery_app
from tasks.crontasks import CodecovCronTask

log = logging.getLogger(__name__)


class FlareCleanupTask(CodecovCronTask, name=flare_cleanup_task_name):
"""
Flare is a field on a Pull object.
Flare is used to draw static graphs (see GraphHandler view in api) and can be large.
The majority of flare graphs are used in pr comments, so we keep the (maybe large) flare "available"
in either the db or Archive storage while the pull is OPEN.
If the pull is not OPEN, we dump the flare to save space.
If we need to generate a flare graph for a non-OPEN pull, we build_report_from_commit
and generate fresh flare from that report (see GraphHandler view in api).
"""

@classmethod
def get_min_seconds_interval_between_executions(cls):
return 72000 # 20h

Check warning on line 26 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L26

Added line #L26 was not covered by tests

def run_cron_task(self, db_session, *args, **kwargs):
# for any Pull that is not OPEN, clear the flare field(s)
non_open_pulls = Pull.objects.exclude(state=PullStates.OPEN.value)

Check warning on line 30 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L30

Added line #L30 was not covered by tests

log.info("Starting FlareCleanupTask")

Check warning on line 32 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L32

Added line #L32 was not covered by tests

# clear in db
non_open_pulls_with_flare_in_db = non_open_pulls.filter(

Check warning on line 35 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L35

Added line #L35 was not covered by tests
_flare__isnull=False
).exclude(_flare={})
# single query, objs are not loaded into memory, does not call .save(), does not refresh updatestamp
n_updated = non_open_pulls_with_flare_in_db.update(_flare=None)
log.info(f"FlareCleanupTask cleared {n_updated} _flares")

Check warning on line 40 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L39-L40

Added lines #L39 - L40 were not covered by tests

# clear in Archive
non_open_pulls_with_flare_in_archive = non_open_pulls.filter(

Check warning on line 43 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L43

Added line #L43 was not covered by tests
_flare_storage_path__isnull=False
).select_related("repository")
log.info(

Check warning on line 46 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L46

Added line #L46 was not covered by tests
f"FlareCleanupTask will clear {non_open_pulls_with_flare_in_archive.count()} Archive flares"
)
# single query, loads all pulls and repos in qset into memory, deletes file in Archive 1 by 1
for pull in non_open_pulls_with_flare_in_archive:
archive_service = ArchiveService(repository=pull.repository)
archive_service.delete_file(pull._flare_storage_path)

Check warning on line 52 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L50-L52

Added lines #L50 - L52 were not covered by tests

# single query, objs are not loaded into memory, does not call .save(), does not refresh updatestamp
n_updated = non_open_pulls_with_flare_in_archive.update(

Check warning on line 55 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L55

Added line #L55 was not covered by tests
_flare_storage_path=None
)

log.info(f"FlareCleanupTask cleared {n_updated} Archive flares")

Check warning on line 59 in tasks/flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/flare_cleanup.py#L59

Added line #L59 was not covered by tests


RegisteredFlareCleanupTask = celery_app.register_task(FlareCleanupTask())
flare_cleanup_task = celery_app.tasks[RegisteredFlareCleanupTask.name]
127 changes: 127 additions & 0 deletions tasks/tests/unit/test_flare_cleanup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import json
from unittest.mock import call

from shared.django_apps.core.models import Pull, PullStates
from shared.django_apps.core.tests.factories import PullFactory, RepositoryFactory

from tasks.flare_cleanup import FlareCleanupTask


class TestFlareCleanupTask(object):
def test_get_min_seconds_interval_between_executions(self):
assert isinstance(

Check warning on line 12 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L12

Added line #L12 was not covered by tests
FlareCleanupTask.get_min_seconds_interval_between_executions(),
int,
)
assert FlareCleanupTask.get_min_seconds_interval_between_executions() > 17000

Check warning on line 16 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L16

Added line #L16 was not covered by tests

def test_successful_run(self, transactional_db, mocker):
mock_logs = mocker.patch("logging.Logger.info")
mock_archive_service = mocker.patch(

Check warning on line 20 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L19-L20

Added lines #L19 - L20 were not covered by tests
"shared.django_apps.utils.model_utils.ArchiveService"
)
archive_value_for_flare = {"some": "data"}
mock_archive_service.return_value.read_file.return_value = json.dumps(

Check warning on line 24 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L23-L24

Added lines #L23 - L24 were not covered by tests
archive_value_for_flare
)
mock_path = "path/to/written/object"
mock_archive_service.return_value.write_json_data_to_storage.return_value = (

Check warning on line 28 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L27-L28

Added lines #L27 - L28 were not covered by tests
mock_path
)
mock_archive_service_in_task = mocker.patch(

Check warning on line 31 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L31

Added line #L31 was not covered by tests
"tasks.flare_cleanup.ArchiveService"
)
mock_archive_service_in_task.return_value.delete_file.return_value = None

Check warning on line 34 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L34

Added line #L34 was not covered by tests

local_value_for_flare = {"test": "test"}
open_pull_with_local_flare = PullFactory(

Check warning on line 37 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L36-L37

Added lines #L36 - L37 were not covered by tests
state=PullStates.OPEN.value,
_flare=local_value_for_flare,
repository=RepositoryFactory(),
)
assert open_pull_with_local_flare.flare == local_value_for_flare
assert open_pull_with_local_flare._flare == local_value_for_flare
assert open_pull_with_local_flare._flare_storage_path is None

Check warning on line 44 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L42-L44

Added lines #L42 - L44 were not covered by tests

closed_pull_with_local_flare = PullFactory(

Check warning on line 46 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L46

Added line #L46 was not covered by tests
state=PullStates.CLOSED.value,
_flare=local_value_for_flare,
repository=RepositoryFactory(),
)
assert closed_pull_with_local_flare.flare == local_value_for_flare
assert closed_pull_with_local_flare._flare == local_value_for_flare
assert closed_pull_with_local_flare._flare_storage_path is None

Check warning on line 53 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L51-L53

Added lines #L51 - L53 were not covered by tests

open_pull_with_archive_flare = PullFactory(

Check warning on line 55 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L55

Added line #L55 was not covered by tests
state=PullStates.OPEN.value,
_flare=None,
_flare_storage_path=mock_path,
repository=RepositoryFactory(),
)
assert open_pull_with_archive_flare.flare == archive_value_for_flare
assert open_pull_with_archive_flare._flare is None
assert open_pull_with_archive_flare._flare_storage_path == mock_path

Check warning on line 63 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L61-L63

Added lines #L61 - L63 were not covered by tests

merged_pull_with_archive_flare = PullFactory(

Check warning on line 65 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L65

Added line #L65 was not covered by tests
state=PullStates.MERGED.value,
_flare=None,
_flare_storage_path=mock_path,
repository=RepositoryFactory(),
)
assert merged_pull_with_archive_flare.flare == archive_value_for_flare
assert merged_pull_with_archive_flare._flare is None
assert merged_pull_with_archive_flare._flare_storage_path == mock_path

Check warning on line 73 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L71-L73

Added lines #L71 - L73 were not covered by tests

task = FlareCleanupTask()
task.run_cron_task(transactional_db)

Check warning on line 76 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L75-L76

Added lines #L75 - L76 were not covered by tests

mock_logs.assert_has_calls(

Check warning on line 78 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L78

Added line #L78 was not covered by tests
[
call("Starting FlareCleanupTask"),
call("FlareCleanupTask cleared 1 _flares"),
call("FlareCleanupTask will clear 1 Archive flares"),
call("FlareCleanupTask cleared 1 Archive flares"),
]
)

# there is a cache for flare on the object (all ArchiveFields have this),
# so get a fresh copy of each object without the cached value
open_pull_with_local_flare = Pull.objects.get(id=open_pull_with_local_flare.id)
assert open_pull_with_local_flare.flare == local_value_for_flare
assert open_pull_with_local_flare._flare == local_value_for_flare
assert open_pull_with_local_flare._flare_storage_path is None

Check warning on line 92 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L89-L92

Added lines #L89 - L92 were not covered by tests

closed_pull_with_local_flare = Pull.objects.get(

Check warning on line 94 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L94

Added line #L94 was not covered by tests
id=closed_pull_with_local_flare.id
)
assert closed_pull_with_local_flare.flare == {}
assert closed_pull_with_local_flare._flare is None
assert closed_pull_with_local_flare._flare_storage_path is None

Check warning on line 99 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L97-L99

Added lines #L97 - L99 were not covered by tests

open_pull_with_archive_flare = Pull.objects.get(

Check warning on line 101 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L101

Added line #L101 was not covered by tests
id=open_pull_with_archive_flare.id
)
assert open_pull_with_archive_flare.flare == archive_value_for_flare
assert open_pull_with_archive_flare._flare is None
assert open_pull_with_archive_flare._flare_storage_path == mock_path

Check warning on line 106 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L104-L106

Added lines #L104 - L106 were not covered by tests

merged_pull_with_archive_flare = Pull.objects.get(

Check warning on line 108 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L108

Added line #L108 was not covered by tests
id=merged_pull_with_archive_flare.id
)
assert merged_pull_with_archive_flare.flare == {}
assert merged_pull_with_archive_flare._flare is None
assert merged_pull_with_archive_flare._flare_storage_path is None

Check warning on line 113 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L111-L113

Added lines #L111 - L113 were not covered by tests

mock_logs.reset_mock()

Check warning on line 115 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L115

Added line #L115 was not covered by tests
# check that once these pulls are corrected they are not corrected again
task = FlareCleanupTask()
task.run_cron_task(transactional_db)

Check warning on line 118 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L117-L118

Added lines #L117 - L118 were not covered by tests

mock_logs.assert_has_calls(

Check warning on line 120 in tasks/tests/unit/test_flare_cleanup.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_flare_cleanup.py#L120

Added line #L120 was not covered by tests
[
call("Starting FlareCleanupTask"),
call("FlareCleanupTask cleared 0 _flares"),
call("FlareCleanupTask will clear 0 Archive flares"),
call("FlareCleanupTask cleared 0 Archive flares"),
]
)

0 comments on commit 8b57acf

Please sign in to comment.