diff --git a/readthedocs/projects/signals.py b/readthedocs/projects/signals.py index 877b32dcf47..b67399ff697 100644 --- a/readthedocs/projects/signals.py +++ b/readthedocs/projects/signals.py @@ -3,10 +3,10 @@ import django.dispatch -before_vcs = django.dispatch.Signal(providing_args=['version']) +before_vcs = django.dispatch.Signal(providing_args=['version', 'environmemt']) after_vcs = django.dispatch.Signal(providing_args=['version']) -before_build = django.dispatch.Signal(providing_args=['version']) +before_build = django.dispatch.Signal(providing_args=['version', 'environmemt']) after_build = django.dispatch.Signal(providing_args=['version']) project_import = django.dispatch.Signal(providing_args=['project']) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index e14f93fd599..2874c680908 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -104,20 +104,21 @@ def get_version(version_pk): version_data = api_v2.version(version_pk).get() return APIVersion(**version_data) - def get_vcs_repo(self): - """Get the VCS object of the current project.""" + def get_vcs_repo(self, environment): + """ + Get the VCS object of the current project. + + All VCS commands will be executed using `environment`. + """ version_repo = self.project.vcs_repo( - self.version.slug, - # When called from ``SyncRepositoryTask.run`` we don't have - # a ``setup_env`` so we use just ``None`` and commands won't - # be recorded - getattr(self, 'setup_env', None), + version=self.version.slug, + environment=environment, verbose_name=self.version.verbose_name, version_type=self.version.type ) return version_repo - def sync_repo(self): + def sync_repo(self, environment): """Update the project's repository and hit ``sync_versions`` API.""" # Make Dirs if not os.path.exists(self.project.doc_path): @@ -143,7 +144,7 @@ def sync_repo(self): 'msg': msg, } ) - version_repo = self.get_vcs_repo() + version_repo = self.get_vcs_repo(environment) version_repo.update() self.sync_versions(version_repo) identifier = getattr(self, 'commit', None) or self.version.identifier @@ -238,9 +239,18 @@ def run(self, version_pk): # pylint: disable=arguments-differ try: self.version = self.get_version(version_pk) self.project = self.version.project - before_vcs.send(sender=self.version) + + environment = LocalBuildEnvironment( + project=self.project, + version=self.version, + build=self.build, + record=False, + update_on_success=False, + ) + + before_vcs.send(sender=self.version, environment=environment) with self.project.repo_nonblockinglock(version=self.version): - self.sync_repo() + self.sync_repo(environment) return True except RepositoryError: # Do not log as ERROR handled exceptions @@ -343,6 +353,7 @@ def __init__( if config is not None: self.config = config self.task = task + # TODO: remove this self.setup_env = None # pylint: disable=arguments-differ @@ -437,7 +448,7 @@ def run_setup(self, record=True): Return True if successful. """ - self.setup_env = LocalBuildEnvironment( + environment = LocalBuildEnvironment( project=self.project, version=self.version, build=self.build, @@ -445,15 +456,20 @@ def run_setup(self, record=True): update_on_success=False, ) + # TODO: Remove. + # There is code that still depends of this attribute + # outside this function. Don't use self.setup_env for new code. + self.setup_env = environment + # Environment used for code checkout & initial configuration reading - with self.setup_env: + with environment: try: - before_vcs.send(sender=self.version) + before_vcs.send(sender=self.version, environment=environment) if self.project.skip: raise ProjectBuildsSkippedError try: with self.project.repo_nonblockinglock(version=self.version): - self.setup_vcs() + self.setup_vcs(environment) except vcs_support_utils.LockTimeout as e: self.task.retry(exc=e, throw=False) raise VersionLockedError @@ -467,13 +483,13 @@ def run_setup(self, record=True): ) self.save_build_config() - self.additional_vcs_operations() + self.additional_vcs_operations(environment) finally: after_vcs.send(sender=self.version) - if self.setup_env.failure or self.config is None: + if environment.failure or self.config is None: msg = 'Failing build because of setup failure: {}'.format( - self.setup_env.failure, + environment.failure, ) log.info( LOG_TEMPLATE, @@ -488,23 +504,23 @@ def run_setup(self, record=True): # of VersionLockedError: this exception occurs when a build is # triggered before the previous one has finished (e.g. two webhooks, # one after the other) - if not isinstance(self.setup_env.failure, VersionLockedError): + if not isinstance(environment.failure, VersionLockedError): self.send_notifications(self.version.pk, self.build['id'], email=True) return False - if self.setup_env.successful and not self.project.has_valid_clone: + if environment.successful and not self.project.has_valid_clone: self.set_valid_clone() return True - def additional_vcs_operations(self): + def additional_vcs_operations(self, environment): """ Execution of tasks that involve the project's VCS. All this tasks have access to the configuration object. """ - version_repo = self.get_vcs_repo() + version_repo = self.get_vcs_repo(environment) if version_repo.supports_submodules: version_repo.update_submodules(self.config) @@ -553,6 +569,10 @@ def run_build(self, docker, record): ) try: + before_build.send( + sender=self.version, + environment=self.build_env, + ) with self.project.repo_nonblockinglock(version=self.version): self.setup_python_environment() @@ -660,13 +680,13 @@ def get_build(build_pk): for key, val in build.items() if key not in private_keys } - def setup_vcs(self): + def setup_vcs(self, environment): """ Update the checkout of the repo to make sure it's the latest. This also syncs versions in the DB. """ - self.setup_env.update_build(state=BUILD_STATE_CLONING) + environment.update_build(state=BUILD_STATE_CLONING) log.info( LOG_TEMPLATE, @@ -677,7 +697,7 @@ def setup_vcs(self): } ) try: - self.sync_repo() + self.sync_repo(environment) except RepositoryError: log.warning('There was an error with the repository', exc_info=True) # Re raise the exception to stop the build at this point @@ -988,7 +1008,6 @@ def build_docs(self): :rtype: dict """ self.build_env.update_build(state=BUILD_STATE_BUILDING) - before_build.send(sender=self.version) outcomes = defaultdict(lambda: False) outcomes['html'] = self.build_docs_html() diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index 980bde03f90..bd0c8b0e63f 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -3,19 +3,23 @@ from os.path import exists from tempfile import mkdtemp +from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import User from django_dynamic_fixture import get from messages_extends.models import Message from mock import MagicMock, patch -from allauth.socialaccount.models import SocialAccount - -from readthedocs.builds.constants import LATEST, BUILD_STATUS_SUCCESS, EXTERNAL -from readthedocs.builds.models import Build +from readthedocs.builds.constants import ( + BUILD_STATE_TRIGGERED, + BUILD_STATUS_SUCCESS, + EXTERNAL, + LATEST, +) +from readthedocs.builds.models import Build, Version +from readthedocs.doc_builder.environments import LocalBuildEnvironment from readthedocs.doc_builder.exceptions import VersionLockedError -from readthedocs.projects import tasks -from readthedocs.builds.models import Version from readthedocs.oauth.models import RemoteRepository +from readthedocs.projects import tasks from readthedocs.projects.exceptions import RepositoryError from readthedocs.projects.models import Project from readthedocs.rtd_tests.base import RTDTestCase @@ -51,6 +55,22 @@ def setUp(self): ) self.project.users.add(self.eric) + def get_update_docs_task(self, version): + build_env = LocalBuildEnvironment( + version.project, version, record=False, + ) + + update_docs = tasks.UpdateDocsTaskStep( + build_env=build_env, + project=version.project, + version=version, + build={ + 'id': 99, + 'state': BUILD_STATE_TRIGGERED, + }, + ) + return update_docs + def tearDown(self): shutil.rmtree(self.repo) super().tearDown() @@ -236,18 +256,16 @@ def test_check_duplicate_reserved_version_latest(self, checkout_path, api_v2): checkout_path.return_value = local_repo version = self.project.versions.get(slug=LATEST) - sync_repository = tasks.UpdateDocsTaskStep() - sync_repository.version = version - sync_repository.project = self.project + sync_repository = self.get_update_docs_task(version) with self.assertRaises(RepositoryError) as e: - sync_repository.sync_repo() + sync_repository.sync_repo(sync_repository.build_env) self.assertEqual( str(e.exception), RepositoryError.DUPLICATED_RESERVED_VERSIONS, ) delete_git_branch(self.repo, 'latest') - sync_repository.sync_repo() + sync_repository.sync_repo(sync_repository.build_env) api_v2.project().sync_versions.post.assert_called() @patch('readthedocs.projects.tasks.api_v2') @@ -262,11 +280,9 @@ def test_check_duplicate_reserved_version_stable(self, checkout_path, api_v2): checkout_path.return_value = local_repo version = self.project.versions.get(slug=LATEST) - sync_repository = tasks.UpdateDocsTaskStep() - sync_repository.version = version - sync_repository.project = self.project + sync_repository = self.get_update_docs_task(version) with self.assertRaises(RepositoryError) as e: - sync_repository.sync_repo() + sync_repository.sync_repo(sync_repository.build_env) self.assertEqual( str(e.exception), RepositoryError.DUPLICATED_RESERVED_VERSIONS, @@ -281,11 +297,10 @@ def test_check_duplicate_no_reserved_version(self, api_v2): create_git_tag(self.repo, 'no-reserved') version = self.project.versions.get(slug=LATEST) - sync_repository = tasks.UpdateDocsTaskStep() - sync_repository.version = version - sync_repository.project = self.project - sync_repository.sync_repo() + sync_repository = self.get_update_docs_task(version) + + sync_repository.sync_repo(sync_repository.build_env) api_v2.project().sync_versions.post.assert_called() def test_public_task_exception(self): diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 1db2f85688f..c45e3093156 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -8,7 +8,7 @@ from django_dynamic_fixture import get from mock import MagicMock, PropertyMock, patch -from readthedocs.builds.constants import EXTERNAL, BUILD_STATE_TRIGGERED +from readthedocs.builds.constants import BUILD_STATE_TRIGGERED, EXTERNAL from readthedocs.builds.models import Version from readthedocs.config import ( ALL, @@ -20,12 +20,12 @@ from readthedocs.config.models import PythonInstallRequirements from readthedocs.config.tests.utils import apply_fs from readthedocs.doc_builder.config import load_yaml_config +from readthedocs.doc_builder.constants import DOCKER_IMAGE_SETTINGS from readthedocs.doc_builder.environments import LocalBuildEnvironment from readthedocs.doc_builder.python_environments import Conda, Virtualenv from readthedocs.projects import tasks from readthedocs.projects.models import Project from readthedocs.rtd_tests.utils import create_git_submodule, make_git_repo -from readthedocs.doc_builder.constants import DOCKER_IMAGE_SETTINGS def create_load(config=None): @@ -1062,7 +1062,7 @@ def test_submodules_include( update_docs = self.get_update_docs_task() checkout_path.return_value = git_repo - update_docs.additional_vcs_operations() + update_docs.additional_vcs_operations(update_docs.build_env) args, kwargs = checkout_submodules.call_args assert set(args[0]) == set(expected) @@ -1091,7 +1091,7 @@ def test_submodules_exclude( update_docs = self.get_update_docs_task() checkout_path.return_value = git_repo - update_docs.additional_vcs_operations() + update_docs.additional_vcs_operations(update_docs.build_env) args, kwargs = checkout_submodules.call_args assert set(args[0]) == {'two', 'three'} @@ -1120,7 +1120,7 @@ def test_submodules_exclude_all( update_docs = self.get_update_docs_task() checkout_path.return_value = git_repo - update_docs.additional_vcs_operations() + update_docs.additional_vcs_operations(update_docs.build_env) checkout_submodules.assert_not_called() @@ -1143,6 +1143,6 @@ def test_submodules_default_exclude_all( update_docs = self.get_update_docs_task() checkout_path.return_value = git_repo - update_docs.additional_vcs_operations() + update_docs.additional_vcs_operations(update_docs.build_env) checkout_submodules.assert_not_called()