Merge pull request #4900 from dojutsu-user/implement-get-objects-or-log

Implement get objects or log

Author: ericholscher

readthedocs/builds/managers.py

@@ -2,7 +2,10 @@
 
 """Build and Version class model Managers."""
 
+import logging
+
 from django.db import models
+from django.core.exceptions import ObjectDoesNotExist
 
 from readthedocs.core.utils.extend import (
     SettingsOverrideObject,
@@ -19,6 +22,8 @@
 )
 from .querysets import VersionQuerySet
 
+log = logging.getLogger(__name__)
+
 
 __all__ = ['VersionManager']
 
@@ -67,6 +72,18 @@ def create_latest(self, **kwargs):
         defaults.update(kwargs)
         return self.create(**defaults)
 
+    def get_object_or_log(self, **kwargs):
+        """
+        Returns Version object or log.
+
+        It will return the Version object if found for the given kwargs,
+        otherwise it will log a warning along with all provided kwargs.
+        """
+        try:
+            return super().get(**kwargs)
+        except ObjectDoesNotExist:
+            log.warning('Version not found for given kwargs. %s' % kwargs)
+
 
 class VersionManager(SettingsOverrideObject):
     _default_class = VersionManagerBase

readthedocs/projects/tasks.py

@@ -903,7 +903,9 @@ def sync_files(
     synchronization of build artifacts on each application instance.
     """
     # Clean up unused artifacts
-    version = Version.objects.get(pk=version_pk)
+    version = Version.objects.get_object_or_log(pk=version_pk)
+    if not version:
+        return
     if not pdf:
         remove_dirs([
             version.project.get_production_media_path(
@@ -965,7 +967,9 @@ def move_files(
     :param epub: Sync ePub files
     :type epub: bool
     """
-    version = Version.objects.get(pk=version_pk)
+    version = Version.objects.get_object_or_log(pk=version_pk)
+    if not version:
+        return
     log.debug(
         LOG_TEMPLATE.format(
             project=version.project.slug,
@@ -1040,7 +1044,9 @@ def update_search(version_pk, commit, delete_non_commit_files=True):
     :param commit: Commit that updated index
     :param delete_non_commit_files: Delete files not in commit from index
     """
-    version = Version.objects.get(pk=version_pk)
+    version = Version.objects.get_object_or_log(pk=version_pk)
+    if not version:
+        return
 
     page_list = process_all_json_files(version, build_dir=False)
 
@@ -1135,7 +1141,9 @@ def fileify(version_pk, commit):
 
     This is so we have an idea of what files we have in the database.
     """
-    version = Version.objects.get(pk=version_pk)
+    version = Version.objects.get_object_or_log(pk=version_pk)
+    if not version:
+        return
     project = version.project
 
     if not commit:
@@ -1225,7 +1233,9 @@ def _manage_imported_files(version, path, commit):
 
 @app.task(queue='web')
 def send_notifications(version_pk, build_pk):
-    version = Version.objects.get(pk=version_pk)
+    version = Version.objects.get_object_or_log(pk=version_pk)
+    if not version:
+        return
     build = Build.objects.get(pk=build_pk)
 
     for hook in version.project.webhook_notifications.all():

readthedocs/rtd_tests/tests/test_build_notifications.py

@@ -17,6 +17,13 @@ def setUp(self):
         self.project = fixture.get(Project)
         self.version = fixture.get(Version, project=self.project)
         self.build = fixture.get(Build, version=self.version)
+    
+    @patch('readthedocs.builds.managers.log')
+    def test_send_notification_none_if_wrong_version_pk(self, mock_logger):
+        self.assertFalse(Version.objects.filter(pk=345343).exists())
+        send_notifications(version_pk=345343, build_pk=None)
+        mock_logger.warning.assert_called_with("Version not found for given kwargs. {'pk': 345343}")
+
 
     def test_send_notification_none(self):
         send_notifications(self.version.pk, self.build.pk)

readthedocs/rtd_tests/tests/test_celery.py

@@ -12,6 +12,7 @@
 from readthedocs.builds.models import Build
 from readthedocs.doc_builder.exceptions import VersionLockedError
 from readthedocs.projects import tasks
+from readthedocs.builds.models import Version
 from readthedocs.projects.exceptions import RepositoryError
 from readthedocs.projects.models import Project
 from readthedocs.rtd_tests.base import RTDTestCase
@@ -250,3 +251,27 @@ def public_task_exception():
                 'error': 'Something bad happened',
             },
         )
+
+    @patch('readthedocs.builds.managers.log')
+    def test_sync_files_logging_when_wrong_version_pk(self, mock_logger):
+        self.assertFalse(Version.objects.filter(pk=345343).exists())
+        tasks.sync_files(project_pk=None, version_pk=345343)
+        mock_logger.warning.assert_called_with("Version not found for given kwargs. {'pk': 345343}")
+
+    @patch('readthedocs.builds.managers.log')
+    def test_move_files_logging_when_wrong_version_pk(self, mock_logger):
+        self.assertFalse(Version.objects.filter(pk=345343).exists())
+        tasks.move_files(version_pk=345343, hostname=None)
+        mock_logger.warning.assert_called_with("Version not found for given kwargs. {'pk': 345343}")
+
+    @patch('readthedocs.builds.managers.log')
+    def test_update_search_logging_when_wrong_version_pk(self, mock_logger):
+        self.assertFalse(Version.objects.filter(pk=345343).exists())
+        tasks.update_search(version_pk=345343, commit=None)
+        mock_logger.warning.assert_called_with("Version not found for given kwargs. {'pk': 345343}")
+
+    @patch('readthedocs.builds.managers.log')
+    def test_fileify_logging_when_wrong_version_pk(self, mock_logger):
+        self.assertFalse(Version.objects.filter(pk=345343).exists())
+        tasks.fileify(version_pk=345343, commit=None)
+        mock_logger.warning.assert_called_with("Version not found for given kwargs. {'pk': 345343}")