From 3d6b60547042eda51cd07cb34e122f067f1a5784 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 17 Aug 2021 17:14:08 +0200 Subject: [PATCH 1/4] Track organization artifacts cleanup Add an extra field on the `Organization` model to keep track if its artificats were already cleanup. This allows us to filter disabled organization and run the cleaning periodically. The new field `Organization.artifacts_cleaned` is automatically set as `False` when a new build happens for any Project under the organization and is set to `True` when running a Django Admin action over the organization. --- .../migrations/0006_add_assets_cleaned.py | 23 +++++++++++++++++++ readthedocs/organizations/models.py | 5 ++++ readthedocs/organizations/signals.py | 13 ++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 readthedocs/organizations/migrations/0006_add_assets_cleaned.py diff --git a/readthedocs/organizations/migrations/0006_add_assets_cleaned.py b/readthedocs/organizations/migrations/0006_add_assets_cleaned.py new file mode 100644 index 00000000000..2a6ead569d0 --- /dev/null +++ b/readthedocs/organizations/migrations/0006_add_assets_cleaned.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2.24 on 2021-08-17 14:54 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('organizations', '0005_historicalorganization_historicalteam'), + ] + + operations = [ + migrations.AddField( + model_name='historicalorganization', + name='artifacts_cleaned', + field=models.BooleanField(default=False, help_text='Artifacts are cleaned out from storage', verbose_name='Artifacts Cleaned'), + ), + migrations.AddField( + model_name='organization', + name='artifacts_cleaned', + field=models.BooleanField(default=False, help_text='Artifacts are cleaned out from storage', verbose_name='Artifacts Cleaned'), + ), + ] diff --git a/readthedocs/organizations/models.py b/readthedocs/organizations/models.py index 9e9ca0b883c..69360e6bcbd 100644 --- a/readthedocs/organizations/models.py +++ b/readthedocs/organizations/models.py @@ -76,6 +76,11 @@ class Organization(models.Model): help_text='Docs and builds are disabled for this organization', default=False, ) + artifacts_cleaned = models.BooleanField( + _('Artifacts Cleaned'), + help_text='Artifacts are cleaned out from storage', + default=False, + ) max_concurrent_builds = models.IntegerField( _('Maximum concurrent builds allowed for this organization'), null=True, diff --git a/readthedocs/organizations/signals.py b/readthedocs/organizations/signals.py index 909d71316cf..635920b8fec 100644 --- a/readthedocs/organizations/signals.py +++ b/readthedocs/organizations/signals.py @@ -7,7 +7,9 @@ from django.db.models.signals import pre_delete from django.dispatch import receiver -from readthedocs.builds.models import Version +from readthedocs.builds.constants import BUILD_STATE_FINISHED +from readthedocs.builds.models import Build, Version +from readthedocs.builds.signals import build_complete from readthedocs.organizations.models import ( Organization, Team, @@ -74,3 +76,12 @@ def remove_organization_completely(sender, instance, using, **kwargs): version.delete() projects.delete() + + +@receiver(build_complete, sender=Build) +def mark_organization_assets_not_cleaned(sender, build, **kwargs): + """Mark the organization assets as not cleaned if there is a new build.""" + organization = build.project.organizations.first() + if build.state == BUILD_STATE_FINISHED and build.success and organization: + organization.artifacts_cleaned = False + organization.save() From 623d7158702585ef46de16ec5089b44b0d7b0cf0 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 23 Aug 2021 13:48:23 +0200 Subject: [PATCH 2/4] Use Celery task to mark organization assets as not cleaned --- readthedocs/organizations/signals.py | 14 +++++++++++--- readthedocs/organizations/tasks.py | 13 +++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 readthedocs/organizations/tasks.py diff --git a/readthedocs/organizations/signals.py b/readthedocs/organizations/signals.py index 635920b8fec..a1d7c834ec7 100644 --- a/readthedocs/organizations/signals.py +++ b/readthedocs/organizations/signals.py @@ -18,6 +18,8 @@ ) from readthedocs.projects.models import Project +from .tasks import mark_organization_assets_not_cleaned as mark_organization_assets_not_cleaned_task + log = logging.getLogger(__name__) @@ -80,8 +82,14 @@ def remove_organization_completely(sender, instance, using, **kwargs): @receiver(build_complete, sender=Build) def mark_organization_assets_not_cleaned(sender, build, **kwargs): - """Mark the organization assets as not cleaned if there is a new build.""" + """ + Mark the organization assets as not cleaned if there is a new build. + + This signal triggers a Celery task because the `build_complete` signal is + fired by the builder and it does not have access to the database. So, we + trigger a Celery task that will be executed in the web and mark the + organization assets as not cleaned. + """ organization = build.project.organizations.first() if build.state == BUILD_STATE_FINISHED and build.success and organization: - organization.artifacts_cleaned = False - organization.save() + mark_organization_assets_not_cleaned_task.delay(build) diff --git a/readthedocs/organizations/tasks.py b/readthedocs/organizations/tasks.py new file mode 100644 index 00000000000..4d026993387 --- /dev/null +++ b/readthedocs/organizations/tasks.py @@ -0,0 +1,13 @@ +import logging + +from readthedocs.worker import app + + +log = logging.getLogger(__name__) + + +@app.task(queue='web') +def mark_organization_assets_not_cleaned(organization): + log.info("Marking organization as not cleaned. organization=%s", organization.slug) + organization.artifacts_cleaned = False + organization.save() From 46d63efa659284f40cf9f4e486885c8c0d55e7e2 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 23 Aug 2021 13:53:10 +0200 Subject: [PATCH 3/4] Handle Build as a dict (APIBuild) --- readthedocs/organizations/signals.py | 5 ++--- readthedocs/organizations/tasks.py | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/readthedocs/organizations/signals.py b/readthedocs/organizations/signals.py index a1d7c834ec7..0896e14e3e6 100644 --- a/readthedocs/organizations/signals.py +++ b/readthedocs/organizations/signals.py @@ -90,6 +90,5 @@ def mark_organization_assets_not_cleaned(sender, build, **kwargs): trigger a Celery task that will be executed in the web and mark the organization assets as not cleaned. """ - organization = build.project.organizations.first() - if build.state == BUILD_STATE_FINISHED and build.success and organization: - mark_organization_assets_not_cleaned_task.delay(build) + if build['state'] == BUILD_STATE_FINISHED: + mark_organization_assets_not_cleaned_task.delay(build['id']) diff --git a/readthedocs/organizations/tasks.py b/readthedocs/organizations/tasks.py index 4d026993387..51c01ce5b34 100644 --- a/readthedocs/organizations/tasks.py +++ b/readthedocs/organizations/tasks.py @@ -7,7 +7,15 @@ @app.task(queue='web') -def mark_organization_assets_not_cleaned(organization): - log.info("Marking organization as not cleaned. organization=%s", organization.slug) - organization.artifacts_cleaned = False - organization.save() +def mark_organization_assets_not_cleaned(build_pk): + try: + build = Build.objects.get(pk=build_pk) + except Build.DoesNotExist: + log.info("Build does not exist. build=%s", build_pk) + return + + organization = build.project.organizations.first() + if organization: + log.info("Marking organization as not cleaned. organization=%s", organization.slug) + organization.artifacts_cleaned = False + organization.save() From c95224f888aaf557c1abbde37f0087deb886e3f6 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 23 Aug 2021 17:00:47 +0200 Subject: [PATCH 4/4] Lint --- readthedocs/organizations/tasks.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/readthedocs/organizations/tasks.py b/readthedocs/organizations/tasks.py index 51c01ce5b34..3210982c73b 100644 --- a/readthedocs/organizations/tasks.py +++ b/readthedocs/organizations/tasks.py @@ -1,5 +1,8 @@ +"""Organization's tasks related.""" + import logging +from readthedocs.builds.models import Build from readthedocs.worker import app @@ -8,6 +11,7 @@ @app.task(queue='web') def mark_organization_assets_not_cleaned(build_pk): + """Mark an organization as `artifacts_cleaned=False`.""" try: build = Build.objects.get(pk=build_pk) except Build.DoesNotExist: