Merge pull request #7678 from readthedocs/delete-external-after-x-days

External versions: delete after 3 months of being merged/closed

docs/guides/autobuild-docs-for-pull-requests.rst

@@ -58,6 +58,8 @@ Searches will default to the default experience for your tool.
 This is a feature we plan to add,
 but don't want to overwhelm our search indexes used in production.
 
+The built documentation is kept for 90 days after the pull/merge request has been closed or merged.
+
 Troubleshooting
 ---------------
 

readthedocs/api/v2/views/integrations.py

@@ -21,14 +21,13 @@
 )
 from readthedocs.core.views.hooks import (
     build_branches,
-    trigger_sync_versions,
-    get_or_create_external_version,
-    delete_external_version,
     build_external_version,
+    deactivate_external_version,
+    get_or_create_external_version,
+    trigger_sync_versions,
 )
 from readthedocs.integrations.models import HttpExchange, Integration
-from readthedocs.projects.models import Project, Feature
-
+from readthedocs.projects.models import Feature, Project
 
 log = logging.getLogger(__name__)
 
@@ -243,14 +242,14 @@ def get_external_version_response(self, project):
             'versions': [to_build],
         }
 
-    def get_delete_external_version_response(self, project):
+    def get_deactivated_external_version_response(self, project):
         """
-        Delete External version on pull/merge request `closed` events and return API response.
+        Deactivate the external version on merge/close events and return the API response.
 
         Return a JSON response with the following::
 
             {
-                "version_deleted": true,
+                "version_deactivated": true,
                 "project": "project_name",
                 "versions": [verbose_name]
             }
@@ -259,14 +258,13 @@ def get_delete_external_version_response(self, project):
         :type project: Project
         """
         identifier, verbose_name = self.get_external_version_data()
-        # Delete external version
-        deleted_version = delete_external_version(
+        deactivated_version = deactivate_external_version(
             project, identifier, verbose_name
         )
         return {
-            'version_deleted': deleted_version is not None,
+            'version_deactivated': bool(deactivated_version),
             'project': project.slug,
-            'versions': [deleted_version],
+            'versions': [deactivated_version] if deactivated_version else [],
         }
 
 
@@ -430,7 +428,7 @@ def handle_webhook(self):
 
             if action == GITHUB_PULL_REQUEST_CLOSED:
                 # Delete external version when PR is closed
-                return self.get_delete_external_version_response(self.project)
+                return self.get_deactivated_external_version_response(self.project)
 
         # Sync versions when push event is created/deleted action
         if all([
@@ -589,7 +587,7 @@ def handle_webhook(self):
 
             if action in [GITLAB_MERGE_REQUEST_CLOSE, GITLAB_MERGE_REQUEST_MERGE]:
                 # Handle merge and close merge_request event.
-                return self.get_delete_external_version_response(self.project)
+                return self.get_deactivated_external_version_response(self.project)
         return None
 
     def _normalize_ref(self, ref):

readthedocs/builds/models.py

@@ -345,9 +345,7 @@ def get_absolute_url(self):
     def delete(self, *args, **kwargs):  # pylint: disable=arguments-differ
         from readthedocs.projects import tasks
         log.info('Removing files for version %s', self.slug)
-        # Remove resources if the version is not external
-        if self.type != EXTERNAL:
-            tasks.clean_project_resources(self.project, self)
+        tasks.clean_project_resources(self.project, self)
         super().delete(*args, **kwargs)
 
     @property

readthedocs/builds/tasks.py

@@ -8,9 +8,15 @@
 from django.core.files.storage import get_storage_class
 
 from readthedocs.api.v2.serializers import BuildSerializer
-from readthedocs.builds.constants import MAX_BUILD_COMMAND_SIZE
+from readthedocs.builds.constants import (
+    BUILD_STATUS_FAILURE,
+    BUILD_STATUS_PENDING,
+    BUILD_STATUS_SUCCESS,
+    MAX_BUILD_COMMAND_SIZE,
+)
 from readthedocs.builds.models import Build, Version
 from readthedocs.builds.utils import memcache_lock
+from readthedocs.projects.tasks import send_build_status
 from readthedocs.worker import app
 
 log = logging.getLogger(__name__)
@@ -163,3 +169,40 @@ def archive_builds_task(days=14, limit=200, include_cold=False, delete=False):
                     build.commands.all().delete()
             except IOError:
                 log.exception('Cold Storage save failure')
+
+
+def delete_inactive_external_versions(limit=200, days=30 * 3):
+    """
+    Delete external versions that have been marked as inactive after ``days``.
+
+    The commit status is updated to link to the build page, as the docs are removed.
+    """
+    days_ago = datetime.now() - timedelta(days=days)
+    queryset = Version.external.filter(
+        active=False,
+        modified__lte=days_ago,
+    )[:limit]
+    for version in queryset:
+        try:
+            last_build = version.last_build
+            if last_build:
+                status = BUILD_STATUS_PENDING
+                if last_build.finished:
+                    status = BUILD_STATUS_SUCCESS if last_build.success else BUILD_STATUS_FAILURE
+                send_build_status(
+                    build_pk=last_build.pk,
+                    commit=last_build.commit,
+                    status=status,
+                    link_to_build=True,
+                )
+        except Exception:
+            log.exception(
+                "Failed to send status: project=%s version=%s",
+                version.project.slug, version.slug,
+            )
+        else:
+            log.info(
+                "Removing external version. project=%s version=%s",
+                version.project.slug, version.slug,
+            )
+            version.delete()

readthedocs/builds/tests/test_tasks.py

@@ -0,0 +1,70 @@
+from datetime import datetime, timedelta
+
+from django.test import TestCase
+from django_dynamic_fixture import get
+
+from readthedocs.builds.constants import BRANCH, EXTERNAL, TAG
+from readthedocs.builds.models import Version
+from readthedocs.builds.tasks import delete_inactive_external_versions
+from readthedocs.projects.models import Project
+
+
+class TestTasks(TestCase):
+
+    def test_delete_inactive_external_versions(self):
+        project = get(Project)
+        project.versions.all().delete()
+        get(
+            Version,
+            project=project,
+            slug='branch',
+            type=BRANCH,
+            active=False,
+            modified=datetime.now() - timedelta(days=7),
+        )
+        get(
+            Version,
+            project=project,
+            slug='tag',
+            type=TAG,
+            active=True,
+            modified=datetime.now() - timedelta(days=7),
+        )
+        get(
+            Version,
+            project=project,
+            slug='external-active',
+            type=EXTERNAL,
+            active=True,
+            modified=datetime.now() - timedelta(days=7),
+        )
+        get(
+            Version,
+            project=project,
+            slug='external-inactive',
+            type=EXTERNAL,
+            active=False,
+            modified=datetime.now() - timedelta(days=3),
+        )
+        get(
+            Version,
+            project=project,
+            slug='external-inactive-old',
+            type=EXTERNAL,
+            active=False,
+            modified=datetime.now() - timedelta(days=7),
+        )
+
+        self.assertEqual(Version.objects.all().count(), 5)
+        self.assertEqual(Version.external.all().count(), 3)
+
+        # We don't have inactive external versions from 9 days ago.
+        delete_inactive_external_versions(days=9)
+        self.assertEqual(Version.objects.all().count(), 5)
+        self.assertEqual(Version.external.all().count(), 3)
+
+        # We have one inactive external versions from 6 days ago.
+        delete_inactive_external_versions(days=6)
+        self.assertEqual(Version.objects.all().count(), 4)
+        self.assertEqual(Version.external.all().count(), 2)
+        self.assertFalse(Version.objects.filter(slug='external-inactive-old').exists())

readthedocs/core/views/hooks.py

@@ -4,10 +4,9 @@
 
 from readthedocs.builds.constants import EXTERNAL
 from readthedocs.core.utils import trigger_build
-from readthedocs.projects.models import Project, Feature
+from readthedocs.projects.models import Feature, Project
 from readthedocs.projects.tasks import sync_repository_task
 
-
 log = logging.getLogger(__name__)
 
 
@@ -139,46 +138,49 @@ def get_or_create_external_version(project, identifier, verbose_name):
 
     if created:
         log.info(
-            '(Create External Version) Added Version: [%s] ',
-            external_version.slug
+            'External version created. project=%s version=%s',
+            project.slug, external_version.slug,
         )
     else:
-        # identifier will change if there is a new commit to the Pull/Merge Request
-        if external_version.identifier != identifier:
-            external_version.identifier = identifier
-            external_version.save()
+        # Identifier will change if there is a new commit to the Pull/Merge Request.
+        external_version.identifier = identifier
+        # If the PR was previously closed it was marked as inactive.
+        external_version.active = True
+        external_version.save()
 
-            log.info(
-                '(Update External Version) Updated Version: [%s] ',
-                external_version.slug
-            )
+        log.info(
+            'External version updated: project=%s version=%s',
+            project.slug, external_version.slug,
+        )
     return external_version
 
 
-def delete_external_version(project, identifier, verbose_name):
+def deactivate_external_version(project, identifier, verbose_name):
     """
-    Delete external versions using `identifier` and `verbose_name`.
+    Deactivate external versions using `identifier` and `verbose_name`.
 
     if external version does not exist then returns `None`.
 
+    We mark the version as inactive,
+    so another celery task will remove it after some days.
+
     :param project: Project instance
     :param identifier: Commit Hash
     :param verbose_name: pull/merge request number
-    :returns:  verbose_name (pull/merge request number).
+    :returns: verbose_name (pull/merge request number).
     :rtype: str
     """
     external_version = project.versions(manager=EXTERNAL).filter(
         verbose_name=verbose_name, identifier=identifier
     ).first()
 
     if external_version:
-        # Delete External Version
-        external_version.delete()
+        external_version.active = False
+        external_version.save()
         log.info(
-            '(Delete External Version) Deleted Version: [%s]',
-            external_version.slug
+            'External version marked as inactive. project=%s version=%s',
+            project.slug, external_version.slug,
         )
-
         return external_version.verbose_name
     return None
 

readthedocs/oauth/services/base.py

@@ -297,7 +297,7 @@ def update_webhook(self, project, integration):
         """
         raise NotImplementedError
 
-    def send_build_status(self, build, commit, state):
+    def send_build_status(self, build, commit, state, link_to_build=False):
         """
         Create commit status for project.
 
@@ -307,6 +307,7 @@ def send_build_status(self, build, commit, state):
         :type commit: str
         :param state: build state failure, pending, or success.
         :type state: str
+        :param link_to_build: If true, link to the build page regardless the state.
         :returns: boolean based on commit status creation was successful or not.
         :rtype: Bool
         """

readthedocs/oauth/services/github.py

@@ -421,7 +421,7 @@ def update_webhook(self, project, integration):
         integration.remove_secret()
         return (False, resp)
 
-    def send_build_status(self, build, commit, state):
+    def send_build_status(self, build, commit, state, link_to_build=False):
         """
         Create GitHub commit status for project.
 
@@ -431,6 +431,7 @@ def send_build_status(self, build, commit, state):
         :type state: str
         :param commit: commit sha of the pull request
         :type commit: str
+        :param link_to_build: If true, link to the build page regardless the state.
         :returns: boolean based on commit status creation was successful or not.
         :rtype: Bool
         """
@@ -444,7 +445,7 @@ def send_build_status(self, build, commit, state):
 
         target_url = build.get_full_url()
 
-        if state == BUILD_STATUS_SUCCESS:
+        if not link_to_build and state == BUILD_STATUS_SUCCESS:
             target_url = build.version.get_absolute_url()
 
         context = f'{settings.RTD_BUILD_STATUS_API_NAME}:{project.slug}'

readthedocs/oauth/services/gitlab.py

@@ -509,7 +509,7 @@ def update_webhook(self, project, integration):
         integration.remove_secret()
         return (False, resp)
 
-    def send_build_status(self, build, commit, state):
+    def send_build_status(self, build, commit, state, link_to_build=False):
         """
         Create GitLab commit status for project.
 
@@ -519,6 +519,7 @@ def send_build_status(self, build, commit, state):
         :type state: str
         :param commit: commit sha of the pull request
         :type commit: str
+        :param link_to_build: If true, link to the build page regardless the state.
         :returns: boolean based on commit status creation was successful or not.
         :rtype: Bool
         """
@@ -537,7 +538,7 @@ def send_build_status(self, build, commit, state):
 
         target_url = build.get_full_url()
 
-        if state == BUILD_STATUS_SUCCESS:
+        if not link_to_build and state == BUILD_STATUS_SUCCESS:
             target_url = build.version.get_absolute_url()
 
         context = f'{settings.RTD_BUILD_STATUS_API_NAME}:{project.slug}'

readthedocs/projects/tasks.py

@@ -1892,7 +1892,7 @@ def retry_domain_verification(domain_pk):
 
 
 @app.task(queue='web')
-def send_build_status(build_pk, commit, status):
+def send_build_status(build_pk, commit, status, link_to_build=False):
     """
     Send Build Status to Git Status API for project external versions.
 
@@ -1932,7 +1932,12 @@ def send_build_status(build_pk, commit, status):
 
         if service is not None:
             # Send status report using the API.
-            success = service.send_build_status(build, commit, status)
+            success = service.send_build_status(
+                build=build,
+                commit=commit,
+                state=status,
+                link_to_build=link_to_build,
+            )
 
         if success:
             log.info(