diff --git a/rsconnect/environment.py b/rsconnect/environment.py index 35280fef..5e4de432 100644 --- a/rsconnect/environment.py +++ b/rsconnect/environment.py @@ -101,6 +101,7 @@ def create_python_environment( python: typing.Optional[str] = None, override_python_version: typing.Optional[str] = None, app_file: typing.Optional[str] = None, + require_requirements_txt: bool = True, ) -> "Environment": """Given a project directory and a Python executable, return Environment information. @@ -122,7 +123,7 @@ def create_python_environment( # click.secho(' Deploying %s to server "%s"' % (directory, connect_server.url)) _warn_on_ignored_manifest(directory) - _warn_if_no_requirements_file(directory) + _check_requirements_file(directory, require_requirements_txt) _warn_if_environment_directory(directory) python_version_requirement = pyproject.detect_python_version_requirement(directory) @@ -145,7 +146,7 @@ def create_python_environment( python_version_requirement = f"=={override_python_version}" # with cli_feedback("Inspecting Python environment"): - environment = cls._get_python_env_info(module_file, python, force_generate) + environment = cls._get_python_env_info(module_file, python, force_generate, require_requirements_txt) environment.python_version_requirement = python_version_requirement if override_python_version: @@ -160,7 +161,11 @@ def create_python_environment( @classmethod def _get_python_env_info( - cls, file_name: str, python: typing.Optional[str], force_generate: bool = False + cls, + file_name: str, + python: typing.Optional[str], + force_generate: bool = False, + require_requirements_txt: bool = True, ) -> "Environment": """ Gathers the python and environment information relating to the specified file @@ -175,7 +180,12 @@ def _get_python_env_info( """ python = which_python(python) logger.debug("Python: %s" % python) - environment = cls._inspect_environment(python, os.path.dirname(file_name), force_generate=force_generate) + environment = cls._inspect_environment( + python, + os.path.dirname(file_name), + force_generate=force_generate, + require_requirements_txt=require_requirements_txt, + ) if environment.error: raise RSConnectException(environment.error) logger.debug("Python: %s" % python) @@ -189,6 +199,7 @@ def _inspect_environment( directory: str, force_generate: bool = False, check_output: typing.Callable[..., bytes] = subprocess.check_output, + require_requirements_txt: bool = True, ) -> "Environment": """Run the environment inspector using the specified python binary. @@ -202,8 +213,13 @@ def _inspect_environment( args = [python, "-m", "rsconnect.subprocesses.inspect_environment"] if flags: args.append("-" + "".join(flags)) + + # Add arguments for inspect_environment.py args.append(directory) + if not require_requirements_txt: + args.append("--no-require-requirements") + try: environment_json = check_output(args, text=True) except Exception as e: @@ -293,19 +309,31 @@ def _warn_on_ignored_manifest(directory: str) -> None: ) -def _warn_if_no_requirements_file(directory: str) -> None: +def _check_requirements_file(directory: str, require_requirements_txt: bool = True) -> None: """ Checks for the existence of a file called requirements.txt in the given directory. - If it's not there, a warning will be printed. + Raises RSConnectException if it doesn't exist and require_requirements_txt is True. :param directory: the directory to check in. + :param require_requirements_txt: if True, raises exception when requirements.txt is missing. + if False, prints a warning instead (for backwards compatibility in tests). + :raises: RSConnectException if requirements.txt is missing and require_requirements_txt is True. """ if not os.path.exists(os.path.join(directory, "requirements.txt")): - click.secho( - " Warning: Capturing the environment using 'pip freeze'.\n" - " Consider creating a requirements.txt file instead.", - fg="yellow", - ) + if require_requirements_txt: + raise RSConnectException( + "requirements.txt file is required for deployment. \n" + "Please create a requirements.txt file in your project directory.\n" + "For best results, use a virtual environment and create your requirements.txt file with:\n" + "pip freeze > requirements.txt" + ) + else: + # For backwards compatibility in tests + click.secho( + " Warning: Capturing the environment using 'pip freeze'.\n" + " Consider creating a requirements.txt file instead.", + fg="yellow", + ) def _warn_if_environment_directory(directory: typing.Union[str, pathlib.Path]) -> None: diff --git a/rsconnect/subprocesses/inspect_environment.py b/rsconnect/subprocesses/inspect_environment.py index 97b2c9a4..4dab9c7b 100644 --- a/rsconnect/subprocesses/inspect_environment.py +++ b/rsconnect/subprocesses/inspect_environment.py @@ -60,13 +60,17 @@ class EnvironmentException(Exception): pass -def detect_environment(dirname: str, force_generate: bool = False) -> EnvironmentData: +def detect_environment( + dirname: str, force_generate: bool = False, require_requirements_txt: bool = True +) -> EnvironmentData: """Determine the python dependencies in the environment. - `pip freeze` will be used to introspect the environment. + `pip freeze` will be used to introspect the environment if force_generate=True or if + requirements.txt is missing and require_requirements_txt=False. :param: dirname Directory name :param: force_generate Force the generation of an environment + :param: require_requirements_txt If True, requirements.txt is required; otherwise pip freeze is used as fallback :return: a dictionary containing the package spec filename and contents if successful, or a dictionary containing `error` on failure. """ @@ -74,7 +78,15 @@ def detect_environment(dirname: str, force_generate: bool = False) -> Environmen if force_generate: result = pip_freeze() else: - result = output_file(dirname, "requirements.txt", "pip") or pip_freeze() + # Try to read requirements.txt file + try: + result = output_file(dirname, "requirements.txt", "pip") + except EnvironmentException as e: + # For backwards compatibility in tests + if not require_requirements_txt: + result = pip_freeze() + else: + raise e if result is not None: result["python"] = get_python_version() @@ -121,13 +133,13 @@ def output_file(dirname: str, filename: str, package_manager: str): """Read an existing package spec file. Returns a dictionary containing the filename and contents - if successful, None if the file does not exist, - or a dictionary containing 'error' on failure. + if successful, or raises an EnvironmentException if the file does not exist + or on other failures. """ try: path = os.path.join(dirname, filename) if not os.path.exists(path): - return None + raise EnvironmentException(f"{filename} file is required for deployment") with open(path, "r") as f: data = f.read() @@ -207,16 +219,36 @@ def main(): """ try: if len(sys.argv) < 2: - raise EnvironmentException("Usage: %s [-fc] DIRECTORY" % sys.argv[0]) - # directory is always the last argument - directory = sys.argv[len(sys.argv) - 1] + raise EnvironmentException("Usage: %s [-fc] DIRECTORY [--no-require-requirements]" % sys.argv[0]) + + # Parse arguments flags = "" force_generate = False - if len(sys.argv) > 2: + require_requirements_txt = True + + # Check for flags in first argument + if len(sys.argv) > 2 and sys.argv[1].startswith("-") and not sys.argv[1].startswith("--"): flags = sys.argv[1] - if "f" in flags: - force_generate = True - envinfo = detect_environment(directory, force_generate)._asdict() + if "f" in flags: + force_generate = True + + # Check for --no-require-requirements flag + if "--no-require-requirements" in sys.argv: + require_requirements_txt = False + + # directory is always the first non-flag argument + directory_index = 1 + while directory_index < len(sys.argv) and ( + sys.argv[directory_index].startswith("-") or sys.argv[directory_index] == "--no-require-requirements" + ): + directory_index += 1 + + if directory_index >= len(sys.argv): + raise EnvironmentException("Missing directory argument") + + directory = sys.argv[directory_index] + + envinfo = detect_environment(directory, force_generate, require_requirements_txt)._asdict() if "contents" in envinfo: keepers = list(map(strip_ref, envinfo["contents"].split("\n"))) keepers = [line for line in keepers if not exclude(line)] diff --git a/tests/test_bundle.py b/tests/test_bundle.py index e300954e..2585aaac 100644 --- a/tests/test_bundle.py +++ b/tests/test_bundle.py @@ -79,7 +79,7 @@ def test_make_notebook_source_bundle1(self): # the test environment. Don't do this in the production code, which # runs in the notebook server. We need the introspection to run in # the kernel environment and not the notebook server environment. - environment = Environment.create_python_environment(directory) + environment = Environment.create_python_environment(directory, require_requirements_txt=False) with make_notebook_source_bundle( nb_path, environment, @@ -149,7 +149,7 @@ def test_make_notebook_source_bundle2(self): # the test environment. Don't do this in the production code, which # runs in the notebook server. We need the introspection to run in # the kernel environment and not the notebook server environment. - environment = Environment.create_python_environment(directory) + environment = Environment.create_python_environment(directory, require_requirements_txt=False) with make_notebook_source_bundle( nb_path, @@ -249,7 +249,7 @@ def test_make_quarto_source_bundle_from_simple_project(self): # input file. create_fake_quarto_rendered_output(temp_proj, "myquarto") - environment = Environment.create_python_environment(temp_proj) + environment = Environment.create_python_environment(temp_proj, require_requirements_txt=False) # mock the result of running of `quarto inspect ` inspect = { @@ -346,7 +346,7 @@ def test_make_quarto_source_bundle_from_complex_project(self): create_fake_quarto_rendered_output(site_dir, "index") create_fake_quarto_rendered_output(site_dir, "about") - environment = Environment.create_python_environment(temp_proj) + environment = Environment.create_python_environment(temp_proj, require_requirements_txt=False) # mock the result of running of `quarto inspect ` inspect = { @@ -448,7 +448,7 @@ def test_make_quarto_source_bundle_from_project_with_requirements(self): fp.write("pandas\n") fp.close() - environment = Environment.create_python_environment(temp_proj) + environment = Environment.create_python_environment(temp_proj, require_requirements_txt=False) # mock the result of running of `quarto inspect ` inspect = { diff --git a/tests/test_environment.py b/tests/test_environment.py index 1a1c5a95..c9a9700c 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -37,7 +37,7 @@ def test_get_default_locale(self): self.assertEqual(get_default_locale(lambda: (None, None)), "") def test_file(self): - result = Environment.create_python_environment(get_dir("pip1")) + result = Environment.create_python_environment(get_dir("pip1"), require_requirements_txt=True) self.assertTrue(version_re.match(result.pip)) @@ -59,7 +59,7 @@ def test_file(self): self.assertEqual(expected, result) def test_pip_freeze(self): - result = Environment.create_python_environment(get_dir("pip2")) + result = Environment.create_python_environment(get_dir("pip2"), require_requirements_txt=False) # these are the dependencies declared in our pyproject.toml self.assertIn("six", result.contents) @@ -84,6 +84,14 @@ def test_pip_freeze(self): ) self.assertEqual(expected, result) + def test_missing_requirements_file(self): + """Test that missing requirements.txt raises an exception""" + with tempfile.TemporaryDirectory() as empty_dir: + with self.assertRaises(RSConnectException) as context: + Environment.create_python_environment(empty_dir) + + self.assertIn("requirements.txt file is required", str(context.exception)) + def test_filter_pip_freeze_output(self): raw_stdout = "numpy\npandas\n[notice] A new release of pip is available: 23.1.2 -> 23.3\n\ [notice] To update, run: pip install --upgrade pip" @@ -136,22 +144,30 @@ def test_is_not_executable(self): class TestPythonVersionRequirements: def test_pyproject_toml(self): - env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "using_pyproject")) + env = Environment.create_python_environment( + os.path.join(TESTDATA, "python-project", "using_pyproject"), require_requirements_txt=False + ) assert env.python_interpreter == sys.executable assert env.python_version_requirement == ">=3.8" def test_python_version(self): - env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "using_pyversion")) + env = Environment.create_python_environment( + os.path.join(TESTDATA, "python-project", "using_pyversion"), require_requirements_txt=False + ) assert env.python_interpreter == sys.executable assert env.python_version_requirement == ">=3.8,<3.12" def test_all_of_them(self): - env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "allofthem")) + env = Environment.create_python_environment( + os.path.join(TESTDATA, "python-project", "allofthem"), require_requirements_txt=False + ) assert env.python_interpreter == sys.executable assert env.python_version_requirement == ">=3.8,<3.12" def test_missing(self): - env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "empty")) + env = Environment.create_python_environment( + os.path.join(TESTDATA, "python-project", "empty"), require_requirements_txt=False + ) assert env.python_interpreter == sys.executable assert env.python_version_requirement is None @@ -256,6 +272,7 @@ def fake_inspect_environment( directory, force_generate=False, check_output=subprocess.check_output, + require_requirements_txt=True, ): return expected_environment @@ -272,6 +289,7 @@ def fake_inspect_environment( assert environment.python_interpreter == expected_python assert environment == expected_environment + class TestEnvironmentDeprecations: def test_override_python_version(self): with mock.patch.object(rsconnect.environment.logger, "warning") as mock_warning: diff --git a/tests/test_subprocesses_inspect_environment.py b/tests/test_subprocesses_inspect_environment.py new file mode 100644 index 00000000..191ed42e --- /dev/null +++ b/tests/test_subprocesses_inspect_environment.py @@ -0,0 +1,44 @@ +import tempfile +import pytest +from unittest import mock + +from rsconnect.subprocesses.inspect_environment import ( + output_file, + detect_environment, + EnvironmentException, +) + + +def test_output_file_requires_requirements_txt(): + """Test that output_file raises an exception when requirements.txt is missing""" + with tempfile.TemporaryDirectory() as empty_dir: + with pytest.raises(EnvironmentException) as context: + output_file(empty_dir, "requirements.txt", "pip") + + assert "requirements.txt file is required" in str(context.value) + + +def test_detect_environment_requires_requirements_txt(): + """Test that detect_environment raises an exception when requirements.txt is missing""" + with tempfile.TemporaryDirectory() as empty_dir: + with pytest.raises(EnvironmentException) as context: + detect_environment(empty_dir, force_generate=False) + + assert "requirements.txt file is required" in str(context.value) + + +def test_detect_environment_with_force_generate(): + """Test that detect_environment still works with force_generate=True""" + with tempfile.TemporaryDirectory() as empty_dir: + with mock.patch("rsconnect.subprocesses.inspect_environment.pip_freeze") as mock_pip_freeze: + mock_pip_freeze.return_value = { + "filename": "requirements.txt", + "contents": "numpy\npandas", + "source": "pip_freeze", + "package_manager": "pip", + } + # This should not raise an exception + environment = detect_environment(empty_dir, force_generate=True) + assert environment.filename == "requirements.txt" + assert "numpy" in environment.contents + assert "pandas" in environment.contents