Merge pull request #5765 from stsewd/use-version-to-trigger-tasks

Use version_pk to trigger builds

Author: stsewd

readthedocs/core/management/commands/update_repos.py

@@ -73,9 +73,8 @@ def handle(self, *args, **options):
 
                         # pylint: disable=no-value-for-parameter
                         tasks.update_docs_task(
-                            version.project_id,
+                            version.pk,
                             build_pk=build.pk,
-                            version_pk=version.pk,
                         )
                 else:
                     p = Project.all_objects.get(slug=slug)
@@ -90,15 +89,16 @@ def handle(self, *args, **options):
                 ):
                     # pylint: disable=no-value-for-parameter
                     tasks.update_docs_task(
-                        version.project_id,
+                        version.pk,
                         force=force,
-                        version_pk=version.pk,
                     )
             else:
                 log.info('Updating all docs')
                 for project in Project.objects.all():
                     # pylint: disable=no-value-for-parameter
+                    default_version = project.get_default_version()
+                    version = project.versions.get(slug=default_version)
                     tasks.update_docs_task(
-                        project.pk,
+                        version.pk,
                         force=force,
                     )

readthedocs/core/management/commands/update_versions.py

@@ -16,7 +16,6 @@ def handle(self, *args, **options):
         for version in Version.objects.filter(active=True, built=False):
             # pylint: disable=no-value-for-parameter
             update_docs_task(
-                version.project_id,
+                version.pk,
                 record=False,
-                version_pk=version.pk,
             )

readthedocs/core/utils/__init__.py

@@ -1,5 +1,3 @@
-# -*- coding: utf-8 -*-
-
 """Common utilty functions."""
 
 from __future__ import absolute_import
@@ -97,7 +95,6 @@ def prepare_build(
         version = project.versions.get(slug=default_version)
 
     kwargs = {
-        'version_pk': version.pk,
         'record': record,
         'force': force,
     }
@@ -131,7 +128,7 @@ def prepare_build(
 
     return (
         update_docs_task.signature(
-            args=(project.pk,),
+            args=(version.pk,),
             kwargs=kwargs,
             options=options,
             immutable=True,

readthedocs/projects/tasks.py

@@ -88,25 +88,16 @@ class SyncRepositoryMixin:
     """Mixin that handles the VCS sync/update."""
 
     @staticmethod
-    def get_version(project=None, version_pk=None):
+    def get_version(version_pk):
         """
         Retrieve version data from the API.
 
-        :param project: project object to sync
-        :type project: projects.models.Project
         :param version_pk: version pk to sync
         :type version_pk: int
         :returns: a data-complete version object
         :rtype: builds.models.APIVersion
         """
-        if not (project or version_pk):
-            raise ValueError('project or version_pk is needed')
-        if version_pk:
-            version_data = api_v2.version(version_pk).get()
-        else:
-            version_data = (
-                api_v2.version(project.slug).get(slug=LATEST)['objects'][0]
-            )
+        version_data = api_v2.version(version_pk).get()
         return APIVersion(**version_data)
 
     def get_vcs_repo(self):
@@ -240,7 +231,7 @@ def run(self, version_pk):  # pylint: disable=arguments-differ
         :rtype: bool
         """
         try:
-            self.version = self.get_version(version_pk=version_pk)
+            self.version = self.get_version(version_pk)
             self.project = self.version.project
             self.sync_repo()
             return True
@@ -288,9 +279,9 @@ def run(self, version_pk):  # pylint: disable=arguments-differ
         MkDocsYAMLParseError,
     ),
 )
-def update_docs_task(self, project_id, *args, **kwargs):
+def update_docs_task(self, version_pk, *args, **kwargs):
     step = UpdateDocsTaskStep(task=self)
-    return step.run(project_id, *args, **kwargs)
+    return step.run(version_pk, *args, **kwargs)
 
 
 class UpdateDocsTaskStep(SyncRepositoryMixin):
@@ -340,7 +331,7 @@ def __init__(
 
     # pylint: disable=arguments-differ
     def run(
-            self, pk, version_pk=None, build_pk=None, record=True, docker=None,
+            self, version_pk, build_pk=None, record=True, docker=None,
             force=False, **__
     ):
         """
@@ -359,8 +350,7 @@ def run(
         the user to bug us. It is therefore a benefit to have as few unhandled
         errors as possible.
 
-        :param pk int: Project id
-        :param version_pk int: Project Version id (latest if None)
+        :param version_pk int: Project Version id
         :param build_pk int: Build id (if None, commands are not recorded)
         :param record bool: record a build object in the database
         :param docker bool: use docker to build the project (if ``None``,
@@ -374,8 +364,8 @@ def run(
         try:
             if docker is None:
                 docker = settings.DOCKER_ENABLE
-            self.project = self.get_project(pk)
-            self.version = self.get_version(self.project, version_pk)
+            self.version = self.get_version(version_pk)
+            self.project = self.version.project
             self.build = self.get_build(build_pk)
             self.build_force = force
             self.config = None

readthedocs/rtd_tests/tests/test_core_utils.py

@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 """Test core util functions."""
 
 import os
@@ -50,15 +49,14 @@ def test_trigger_build_when_version_not_provided_default_version_exist(self, upd
 
         trigger_build(project=project_1)
         kwargs = {
-            'version_pk': version_1.pk,
             'record': True,
             'force': False,
             'build_pk': mock.ANY,
         }
 
         update_docs_task.signature.assert_has_calls([
             mock.call(
-                args=(project_1.pk,),
+                args=(version_1.pk,),
                 kwargs=kwargs,
                 options=mock.ANY,
                 immutable=True,
@@ -70,20 +68,19 @@ def test_trigger_build_when_version_not_provided_default_version_doesnt_exist(se
 
         trigger_build(project=self.project)
         default_version = self.project.get_default_version()
-        version_ = self.project.versions.get(slug=default_version)
+        version = self.project.versions.get(slug=default_version)
 
-        self.assertEqual(version_.slug, LATEST)
+        self.assertEqual(version.slug, LATEST)
 
         kwargs = {
-            'version_pk': version_.pk,
             'record': True,
             'force': False,
             'build_pk': mock.ANY,
         }
 
         update_docs_task.signature.assert_has_calls([
             mock.call(
-                args=(self.project.pk,),
+                args=(version.pk,),
                 kwargs=kwargs,
                 options=mock.ANY,
                 immutable=True,
@@ -96,7 +93,6 @@ def test_trigger_custom_queue(self, update_docs):
         self.project.build_queue = 'build03'
         trigger_build(project=self.project, version=self.version)
         kwargs = {
-            'version_pk': self.version.pk,
             'record': True,
             'force': False,
             'build_pk': mock.ANY,
@@ -108,7 +104,7 @@ def test_trigger_custom_queue(self, update_docs):
         }
         update_docs.signature.assert_has_calls([
             mock.call(
-                args=(self.project.pk,),
+                args=(self.version.pk,),
                 kwargs=kwargs,
                 options=options,
                 immutable=True,
@@ -121,7 +117,6 @@ def test_trigger_build_time_limit(self, update_docs):
         """Pass of time limit."""
         trigger_build(project=self.project, version=self.version)
         kwargs = {
-            'version_pk': self.version.pk,
             'record': True,
             'force': False,
             'build_pk': mock.ANY,
@@ -133,7 +128,7 @@ def test_trigger_build_time_limit(self, update_docs):
         }
         update_docs.signature.assert_has_calls([
             mock.call(
-                args=(self.project.pk,),
+                args=(self.version.pk,),
                 kwargs=kwargs,
                 options=options,
                 immutable=True,
@@ -147,7 +142,6 @@ def test_trigger_build_invalid_time_limit(self, update_docs):
         self.project.container_time_limit = '200s'
         trigger_build(project=self.project, version=self.version)
         kwargs = {
-            'version_pk': self.version.pk,
             'record': True,
             'force': False,
             'build_pk': mock.ANY,
@@ -159,7 +153,7 @@ def test_trigger_build_invalid_time_limit(self, update_docs):
         }
         update_docs.signature.assert_has_calls([
             mock.call(
-                args=(self.project.pk,),
+                args=(self.version.pk,),
                 kwargs=kwargs,
                 options=options,
                 immutable=True,
@@ -173,7 +167,6 @@ def test_trigger_build_rounded_time_limit(self, update_docs):
         self.project.container_time_limit = 3
         trigger_build(project=self.project, version=self.version)
         kwargs = {
-            'version_pk': self.version.pk,
             'record': True,
             'force': False,
             'build_pk': mock.ANY,
@@ -185,7 +178,7 @@ def test_trigger_build_rounded_time_limit(self, update_docs):
         }
         update_docs.signature.assert_has_calls([
             mock.call(
-                args=(self.project.pk,),
+                args=(self.version.pk,),
                 kwargs=kwargs,
                 options=options,
                 immutable=True,