Skip to content

Require requirements #680

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
50 changes: 39 additions & 11 deletions rsconnect/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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)
Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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.

Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
58 changes: 45 additions & 13 deletions rsconnect/subprocesses/inspect_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,33 @@ 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.
"""

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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)]
Expand Down
10 changes: 5 additions & 5 deletions tests/test_bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 <project_dir>`
inspect = {
Expand Down Expand Up @@ -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 <project_dir>`
inspect = {
Expand Down Expand Up @@ -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 <project_dir>`
inspect = {
Expand Down
30 changes: 24 additions & 6 deletions tests/test_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand All @@ -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)
Expand All @@ -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"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -256,6 +272,7 @@ def fake_inspect_environment(
directory,
force_generate=False,
check_output=subprocess.check_output,
require_requirements_txt=True,
):
return expected_environment

Expand All @@ -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:
Expand Down
44 changes: 44 additions & 0 deletions tests/test_subprocesses_inspect_environment.py
Original file line number Diff line number Diff line change
@@ -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
Loading