From ac87f67ce50b4873ed95c3c64a2b383c14fbb7f0 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 23 Dec 2024 11:57:07 -0500 Subject: [PATCH] Build: remove conda_append_core_requirements feature flag (#11847) We have had this flag set for a while now. --- .../doc_builder/python_environments.py | 51 ++------------- readthedocs/projects/models.py | 5 -- .../projects/tests/test_build_tasks.py | 65 +++++++------------ 3 files changed, 30 insertions(+), 91 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 3adce28e43c..d21d94aa000 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1878,7 +1878,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" @@ -1909,10 +1908,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."), diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 0f18e53bc7c..32515c1a8aa 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -1552,8 +1552,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, @@ -1588,34 +1593,19 @@ def test_conda_config_calls_conda_command(self, load_yaml_config): mock.call("asdf", "global", "python", python_version), mock.call("asdf", "reshim", "python", record=False), mock.call( - "conda", - "env", - "create", - "--quiet", - "--name", - self.version.slug, - "--file", + "cat", "environment.yaml", cwd=mock.ANY, - bin_path=mock.ANY, ), mock.call( "conda", - "install", - "--yes", + "env", + "create", "--quiet", "--name", self.version.slug, - "sphinx", - cwd=mock.ANY, - ), - mock.call( - mock.ANY, - "-m", - "pip", - "install", - "-U", - "--no-cache-dir", + "--file", + "environment.yaml", cwd=mock.ANY, bin_path=mock.ANY, ), @@ -1654,8 +1644,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, @@ -1683,6 +1678,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", @@ -1695,26 +1695,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), ] )