From 1784f930d88baadc162369945b7e8d2c7400218d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 11 Dec 2024 17:15:21 -0500 Subject: [PATCH 1/4] Build: remove conda_append_core_requirements feature flag --- .../doc_builder/python_environments.py | 51 +++---------------- readthedocs/projects/models.py | 5 -- 2 files changed, 7 insertions(+), 49 deletions(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 06d1b59f82e..e92691c2d83 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -239,9 +239,8 @@ def conda_bin_name(self): return self.config.python_interpreter def setup_base(self): - if self.project.has_feature(Feature.CONDA_APPEND_CORE_REQUIREMENTS): - self._append_core_requirements() - self._show_environment_yaml() + self._append_core_requirements() + self._show_environment_yaml() self.build_env.run( self.conda_bin_name(), @@ -357,48 +356,12 @@ def _get_core_requirements(self): return pip_requirements, conda_requirements def install_core_requirements(self): - """Install basic Read the Docs requirements into the Conda env.""" - - if self.project.has_feature(Feature.CONDA_APPEND_CORE_REQUIREMENTS): - # Skip install core requirements since they were already appended to - # the user's ``environment.yml`` and installed at ``conda env - # create`` step. - return - - pip_requirements, conda_requirements = self._get_core_requirements() - # Install requirements via ``conda install`` command if they were - # not appended to the ``environment.yml`` file. - cmd = [ - self.conda_bin_name(), - "install", - "--yes", - "--quiet", - "--name", - self.version.slug, - ] - cmd.extend(conda_requirements) - self.build_env.run( - *cmd, - cwd=self.checkout_path, - # TODO: on tests I found that we are not passing ``bin_path`` here - # for some reason. - ) + """ + Skip installing requirements. - # Install requirements via ``pip install`` - pip_cmd = [ - self.venv_bin(filename="python"), - "-m", - "pip", - "install", - "-U", - "--no-cache-dir", - ] - pip_cmd.extend(pip_requirements) - self.build_env.run( - *pip_cmd, - bin_path=self.venv_bin(), - cwd=self.checkout_path, # noqa - no comma here in py27 :/ - ) + Skip installing core requirements, since they were already appended to + the user's ``environment.yml`` and installed at ``conda env create`` step. + """ def install_requirements_file(self, install): # as the conda environment was created by using the ``environment.yml`` diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index cce8140a539..65751f18f17 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1917,7 +1917,6 @@ def add_features(sender, **kwargs): # Feature constants - this is not a exhaustive list of features, features # may be added by other packages API_LARGE_DATA = "api_large_data" - CONDA_APPEND_CORE_REQUIREMENTS = "conda_append_core_requirements" RECORD_404_PAGE_VIEWS = "record_404_page_views" DISABLE_PAGEVIEWS = "disable_pageviews" RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header" @@ -1948,10 +1947,6 @@ def add_features(sender, **kwargs): API_LARGE_DATA, _("Build: Try alternative method of posting large data"), ), - ( - CONDA_APPEND_CORE_REQUIREMENTS, - _("Conda: Append Read the Docs core requirements to environment.yml file"), - ), ( RECORD_404_PAGE_VIEWS, _("Proxito: Record 404s page views."), From 373f36c351832fa8346e0b660cdd79fc1fd42432 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 11 Dec 2024 17:37:52 -0500 Subject: [PATCH 2/4] Fix tests --- readthedocs/projects/tests/test_build_tasks.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 7e5639224a3..1c7af79f51a 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -1537,6 +1537,11 @@ def test_conda_config_calls_conda_command(self, load_yaml_config): mock.call("asdf", "install", "python", python_version), mock.call("asdf", "global", "python", python_version), mock.call("asdf", "reshim", "python", record=False), + mock.call( + "cat", + "environment.yaml", + cwd=mock.ANY, + ), mock.call( "conda", "env", @@ -1559,16 +1564,6 @@ def test_conda_config_calls_conda_command(self, load_yaml_config): "sphinx", cwd=mock.ANY, ), - mock.call( - mock.ANY, - "-m", - "pip", - "install", - "-U", - "--no-cache-dir", - cwd=mock.ANY, - bin_path=mock.ANY, - ), mock.call("test", "-x", "_build/html", cwd=mock.ANY, record=False), mock.call("lsb_release", "--description", record=False, demux=True), mock.call("python", "--version", record=False, demux=True), From b0ce7ee004c8935d351a16ea88b71f497c868a08 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 11 Dec 2024 17:49:08 -0500 Subject: [PATCH 3/4] Fix tests again --- readthedocs/projects/tests/test_build_tasks.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 1c7af79f51a..95e7d42e97e 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -1554,16 +1554,6 @@ def test_conda_config_calls_conda_command(self, load_yaml_config): cwd=mock.ANY, bin_path=mock.ANY, ), - mock.call( - "conda", - "install", - "--yes", - "--quiet", - "--name", - self.version.slug, - "sphinx", - cwd=mock.ANY, - ), mock.call("test", "-x", "_build/html", cwd=mock.ANY, record=False), mock.call("lsb_release", "--description", record=False, demux=True), mock.call("python", "--version", record=False, demux=True), From cdafe7cffc573536da7b2dbef8d51dab4ffb26af Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 12 Dec 2024 11:38:42 -0500 Subject: [PATCH 4/4] Fix tests --- .../projects/tests/test_build_tasks.py | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 95e7d42e97e..f1cbd1f6d02 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -1502,8 +1502,13 @@ def test_requirements_from_config_file_installed(self, load_yaml_config): ] ) + @mock.patch("readthedocs.core.utils.filesystem.assert_path_is_inside_docroot") @mock.patch("readthedocs.doc_builder.director.load_yaml_config") - def test_conda_config_calls_conda_command(self, load_yaml_config): + def test_conda_config_calls_conda_command( + self, load_yaml_config, assert_path_is_inside_docroot + ): + # While testing, we are unsure if temporary test files exist in the docroot. + assert_path_is_inside_docroot.return_value = True load_yaml_config.return_value = get_build_config( { "version": 2, @@ -1589,8 +1594,13 @@ def test_conda_config_calls_conda_command(self, load_yaml_config): ], ) + @mock.patch("readthedocs.core.utils.filesystem.assert_path_is_inside_docroot") @mock.patch("readthedocs.doc_builder.director.load_yaml_config") - def test_python_mamba_commands(self, load_yaml_config): + def test_python_mamba_commands( + self, load_yaml_config, assert_path_is_inside_docroot + ): + # While testing, we are unsure if temporary test files exist in the docroot. + assert_path_is_inside_docroot.return_value = True load_yaml_config.return_value = get_build_config( { "version": 2, @@ -1618,6 +1628,11 @@ def test_python_mamba_commands(self, load_yaml_config): mock.call("asdf", "install", "python", "mambaforge-4.10.3-10"), mock.call("asdf", "global", "python", "mambaforge-4.10.3-10"), mock.call("asdf", "reshim", "python", record=False), + mock.call( + "cat", + "environment.yaml", + cwd=mock.ANY, + ), mock.call( "mamba", "env", @@ -1630,26 +1645,7 @@ def test_python_mamba_commands(self, load_yaml_config): bin_path=None, cwd=mock.ANY, ), - mock.call( - "mamba", - "install", - "--yes", - "--quiet", - "--name", - "latest", - "sphinx", - cwd=mock.ANY, - ), - mock.call( - mock.ANY, - "-m", - "pip", - "install", - "-U", - "--no-cache-dir", - bin_path=mock.ANY, - cwd=mock.ANY, - ), + mock.call("test", "-x", "_build/html", cwd=mock.ANY, record=False), ] )