Skip to content

Build: remove conda_append_core_requirements feature flag #11847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 7 additions & 44 deletions readthedocs/doc_builder/python_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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``
Expand Down
5 changes: 0 additions & 5 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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."),
Expand Down
65 changes: 23 additions & 42 deletions readthedocs/projects/tests/test_build_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1538,34 +1543,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,
),
Expand Down Expand Up @@ -1604,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,
Expand Down Expand Up @@ -1633,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",
Expand All @@ -1645,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),
]
)

Expand Down