Skip to content

Commit accfe2f

Browse files
committed
First stab at requireing requirements
1 parent 1108457 commit accfe2f

File tree

5 files changed

+139
-35
lines changed

5 files changed

+139
-35
lines changed

rsconnect/environment.py

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ def create_python_environment(
101101
python: typing.Optional[str] = None,
102102
override_python_version: typing.Optional[str] = None,
103103
app_file: typing.Optional[str] = None,
104+
require_requirements_txt: bool = True,
104105
) -> "Environment":
105106
"""Given a project directory and a Python executable, return Environment information.
106107
@@ -122,7 +123,7 @@ def create_python_environment(
122123

123124
# click.secho(' Deploying %s to server "%s"' % (directory, connect_server.url))
124125
_warn_on_ignored_manifest(directory)
125-
_warn_if_no_requirements_file(directory)
126+
_check_requirements_file(directory, require_requirements_txt)
126127
_warn_if_environment_directory(directory)
127128

128129
python_version_requirement = pyproject.detect_python_version_requirement(directory)
@@ -145,7 +146,7 @@ def create_python_environment(
145146
python_version_requirement = f"=={override_python_version}"
146147

147148
# with cli_feedback("Inspecting Python environment"):
148-
environment = cls._get_python_env_info(module_file, python, force_generate)
149+
environment = cls._get_python_env_info(module_file, python, force_generate, require_requirements_txt)
149150
environment.python_version_requirement = python_version_requirement
150151

151152
if override_python_version:
@@ -160,7 +161,7 @@ def create_python_environment(
160161

161162
@classmethod
162163
def _get_python_env_info(
163-
cls, file_name: str, python: typing.Optional[str], force_generate: bool = False
164+
cls, file_name: str, python: typing.Optional[str], force_generate: bool = False, require_requirements_txt: bool = True
164165
) -> "Environment":
165166
"""
166167
Gathers the python and environment information relating to the specified file
@@ -175,7 +176,7 @@ def _get_python_env_info(
175176
"""
176177
python = which_python(python)
177178
logger.debug("Python: %s" % python)
178-
environment = cls._inspect_environment(python, os.path.dirname(file_name), force_generate=force_generate)
179+
environment = cls._inspect_environment(python, os.path.dirname(file_name), force_generate=force_generate, require_requirements_txt=require_requirements_txt)
179180
if environment.error:
180181
raise RSConnectException(environment.error)
181182
logger.debug("Python: %s" % python)
@@ -189,6 +190,7 @@ def _inspect_environment(
189190
directory: str,
190191
force_generate: bool = False,
191192
check_output: typing.Callable[..., bytes] = subprocess.check_output,
193+
require_requirements_txt: bool = True,
192194
) -> "Environment":
193195
"""Run the environment inspector using the specified python binary.
194196
@@ -202,7 +204,12 @@ def _inspect_environment(
202204
args = [python, "-m", "rsconnect.subprocesses.inspect_environment"]
203205
if flags:
204206
args.append("-" + "".join(flags))
207+
208+
# Add arguments for inspect_environment.py
205209
args.append(directory)
210+
211+
if not require_requirements_txt:
212+
args.append("--no-require-requirements")
206213

207214
try:
208215
environment_json = check_output(args, text=True)
@@ -293,19 +300,31 @@ def _warn_on_ignored_manifest(directory: str) -> None:
293300
)
294301

295302

296-
def _warn_if_no_requirements_file(directory: str) -> None:
303+
def _check_requirements_file(directory: str, require_requirements_txt: bool = True) -> None:
297304
"""
298305
Checks for the existence of a file called requirements.txt in the given directory.
299-
If it's not there, a warning will be printed.
306+
Raises RSConnectException if it doesn't exist and require_requirements_txt is True.
300307
301308
:param directory: the directory to check in.
309+
:param require_requirements_txt: if True, raises exception when requirements.txt is missing.
310+
if False, prints a warning instead (for backwards compatibility in tests).
311+
:raises: RSConnectException if requirements.txt is missing and require_requirements_txt is True.
302312
"""
303313
if not os.path.exists(os.path.join(directory, "requirements.txt")):
304-
click.secho(
305-
" Warning: Capturing the environment using 'pip freeze'.\n"
306-
" Consider creating a requirements.txt file instead.",
307-
fg="yellow",
308-
)
314+
if require_requirements_txt:
315+
raise RSConnectException(
316+
"requirements.txt file is required for deployment. \n"
317+
"Please create a requirements.txt file in your project directory.\n"
318+
"For best results, use a virtual environment and create your requirements.txt file with:\n"
319+
"pip freeze > requirements.txt"
320+
)
321+
else:
322+
# For backwards compatibility in tests
323+
click.secho(
324+
" Warning: Capturing the environment using 'pip freeze'.\n"
325+
" Consider creating a requirements.txt file instead.",
326+
fg="yellow",
327+
)
309328

310329

311330
def _warn_if_environment_directory(directory: typing.Union[str, pathlib.Path]) -> None:

rsconnect/subprocesses/inspect_environment.py

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,31 @@ class EnvironmentException(Exception):
6060
pass
6161

6262

63-
def detect_environment(dirname: str, force_generate: bool = False) -> EnvironmentData:
63+
def detect_environment(dirname: str, force_generate: bool = False, require_requirements_txt: bool = True) -> EnvironmentData:
6464
"""Determine the python dependencies in the environment.
6565
66-
`pip freeze` will be used to introspect the environment.
66+
`pip freeze` will be used to introspect the environment if force_generate=True or if
67+
requirements.txt is missing and require_requirements_txt=False.
6768
6869
:param: dirname Directory name
6970
:param: force_generate Force the generation of an environment
71+
:param: require_requirements_txt If True, requirements.txt is required; otherwise pip freeze is used as fallback
7072
:return: a dictionary containing the package spec filename and contents if successful,
7173
or a dictionary containing `error` on failure.
7274
"""
7375

7476
if force_generate:
7577
result = pip_freeze()
7678
else:
77-
result = output_file(dirname, "requirements.txt", "pip") or pip_freeze()
79+
# Try to read requirements.txt file
80+
try:
81+
result = output_file(dirname, "requirements.txt", "pip")
82+
except EnvironmentException as e:
83+
# For backwards compatibility in tests
84+
if not require_requirements_txt:
85+
result = pip_freeze()
86+
else:
87+
raise e
7888

7989
if result is not None:
8090
result["python"] = get_python_version()
@@ -121,13 +131,13 @@ def output_file(dirname: str, filename: str, package_manager: str):
121131
"""Read an existing package spec file.
122132
123133
Returns a dictionary containing the filename and contents
124-
if successful, None if the file does not exist,
125-
or a dictionary containing 'error' on failure.
134+
if successful, or raises an EnvironmentException if the file does not exist
135+
or on other failures.
126136
"""
127137
try:
128138
path = os.path.join(dirname, filename)
129139
if not os.path.exists(path):
130-
return None
140+
raise EnvironmentException(f"{filename} file is required for deployment")
131141

132142
with open(path, "r") as f:
133143
data = f.read()
@@ -207,16 +217,35 @@ def main():
207217
"""
208218
try:
209219
if len(sys.argv) < 2:
210-
raise EnvironmentException("Usage: %s [-fc] DIRECTORY" % sys.argv[0])
211-
# directory is always the last argument
212-
directory = sys.argv[len(sys.argv) - 1]
220+
raise EnvironmentException("Usage: %s [-fc] DIRECTORY [--no-require-requirements]" % sys.argv[0])
221+
222+
# Parse arguments
213223
flags = ""
214224
force_generate = False
215-
if len(sys.argv) > 2:
225+
require_requirements_txt = True
226+
227+
# Check for flags in first argument
228+
if len(sys.argv) > 2 and sys.argv[1].startswith('-') and not sys.argv[1].startswith('--'):
216229
flags = sys.argv[1]
217-
if "f" in flags:
218-
force_generate = True
219-
envinfo = detect_environment(directory, force_generate)._asdict()
230+
if "f" in flags:
231+
force_generate = True
232+
233+
# Check for --no-require-requirements flag
234+
if "--no-require-requirements" in sys.argv:
235+
require_requirements_txt = False
236+
237+
# directory is always the first non-flag argument
238+
directory_index = 1
239+
while directory_index < len(sys.argv) and (sys.argv[directory_index].startswith('-') or
240+
sys.argv[directory_index] == "--no-require-requirements"):
241+
directory_index += 1
242+
243+
if directory_index >= len(sys.argv):
244+
raise EnvironmentException("Missing directory argument")
245+
246+
directory = sys.argv[directory_index]
247+
248+
envinfo = detect_environment(directory, force_generate, require_requirements_txt)._asdict()
220249
if "contents" in envinfo:
221250
keepers = list(map(strip_ref, envinfo["contents"].split("\n")))
222251
keepers = [line for line in keepers if not exclude(line)]

tests/test_bundle.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def test_make_notebook_source_bundle1(self):
7979
# the test environment. Don't do this in the production code, which
8080
# runs in the notebook server. We need the introspection to run in
8181
# the kernel environment and not the notebook server environment.
82-
environment = Environment.create_python_environment(directory)
82+
environment = Environment.create_python_environment(directory, require_requirements_txt=False)
8383
with make_notebook_source_bundle(
8484
nb_path,
8585
environment,
@@ -149,7 +149,7 @@ def test_make_notebook_source_bundle2(self):
149149
# the test environment. Don't do this in the production code, which
150150
# runs in the notebook server. We need the introspection to run in
151151
# the kernel environment and not the notebook server environment.
152-
environment = Environment.create_python_environment(directory)
152+
environment = Environment.create_python_environment(directory, require_requirements_txt=False)
153153

154154
with make_notebook_source_bundle(
155155
nb_path,
@@ -249,7 +249,7 @@ def test_make_quarto_source_bundle_from_simple_project(self):
249249
# input file.
250250
create_fake_quarto_rendered_output(temp_proj, "myquarto")
251251

252-
environment = Environment.create_python_environment(temp_proj)
252+
environment = Environment.create_python_environment(temp_proj, require_requirements_txt=False)
253253

254254
# mock the result of running of `quarto inspect <project_dir>`
255255
inspect = {
@@ -346,7 +346,7 @@ def test_make_quarto_source_bundle_from_complex_project(self):
346346
create_fake_quarto_rendered_output(site_dir, "index")
347347
create_fake_quarto_rendered_output(site_dir, "about")
348348

349-
environment = Environment.create_python_environment(temp_proj)
349+
environment = Environment.create_python_environment(temp_proj, require_requirements_txt=False)
350350

351351
# mock the result of running of `quarto inspect <project_dir>`
352352
inspect = {
@@ -448,7 +448,7 @@ def test_make_quarto_source_bundle_from_project_with_requirements(self):
448448
fp.write("pandas\n")
449449
fp.close()
450450

451-
environment = Environment.create_python_environment(temp_proj)
451+
environment = Environment.create_python_environment(temp_proj, require_requirements_txt=False)
452452

453453
# mock the result of running of `quarto inspect <project_dir>`
454454
inspect = {

tests/test_environment.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def test_get_default_locale(self):
3737
self.assertEqual(get_default_locale(lambda: (None, None)), "")
3838

3939
def test_file(self):
40-
result = Environment.create_python_environment(get_dir("pip1"))
40+
result = Environment.create_python_environment(get_dir("pip1"), require_requirements_txt=True)
4141

4242
self.assertTrue(version_re.match(result.pip))
4343

@@ -59,7 +59,7 @@ def test_file(self):
5959
self.assertEqual(expected, result)
6060

6161
def test_pip_freeze(self):
62-
result = Environment.create_python_environment(get_dir("pip2"))
62+
result = Environment.create_python_environment(get_dir("pip2"), require_requirements_txt=False)
6363

6464
# these are the dependencies declared in our pyproject.toml
6565
self.assertIn("six", result.contents)
@@ -83,6 +83,14 @@ def test_pip_freeze(self):
8383
python_interpreter=sys.executable,
8484
)
8585
self.assertEqual(expected, result)
86+
87+
def test_missing_requirements_file(self):
88+
"""Test that missing requirements.txt raises an exception"""
89+
with tempfile.TemporaryDirectory() as empty_dir:
90+
with self.assertRaises(RSConnectException) as context:
91+
Environment.create_python_environment(empty_dir)
92+
93+
self.assertIn("requirements.txt file is required", str(context.exception))
8694

8795
def test_filter_pip_freeze_output(self):
8896
raw_stdout = "numpy\npandas\n[notice] A new release of pip is available: 23.1.2 -> 23.3\n\
@@ -136,22 +144,22 @@ def test_is_not_executable(self):
136144

137145
class TestPythonVersionRequirements:
138146
def test_pyproject_toml(self):
139-
env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "using_pyproject"))
147+
env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "using_pyproject"), require_requirements_txt=False)
140148
assert env.python_interpreter == sys.executable
141149
assert env.python_version_requirement == ">=3.8"
142150

143151
def test_python_version(self):
144-
env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "using_pyversion"))
152+
env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "using_pyversion"), require_requirements_txt=False)
145153
assert env.python_interpreter == sys.executable
146154
assert env.python_version_requirement == ">=3.8,<3.12"
147155

148156
def test_all_of_them(self):
149-
env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "allofthem"))
157+
env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "allofthem"), require_requirements_txt=False)
150158
assert env.python_interpreter == sys.executable
151159
assert env.python_version_requirement == ">=3.8,<3.12"
152160

153161
def test_missing(self):
154-
env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "empty"))
162+
env = Environment.create_python_environment(os.path.join(TESTDATA, "python-project", "empty"), require_requirements_txt=False)
155163
assert env.python_interpreter == sys.executable
156164
assert env.python_version_requirement is None
157165

@@ -256,6 +264,7 @@ def fake_inspect_environment(
256264
directory,
257265
force_generate=False,
258266
check_output=subprocess.check_output,
267+
require_requirements_txt=True,
259268
):
260269
return expected_environment
261270

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import os
2+
import tempfile
3+
import pytest
4+
from unittest import mock
5+
6+
from rsconnect.exception import RSConnectException
7+
from rsconnect.subprocesses.inspect_environment import (
8+
output_file,
9+
detect_environment,
10+
pip_freeze,
11+
EnvironmentException,
12+
)
13+
14+
15+
def test_output_file_requires_requirements_txt():
16+
"""Test that output_file raises an exception when requirements.txt is missing"""
17+
with tempfile.TemporaryDirectory() as empty_dir:
18+
with pytest.raises(EnvironmentException) as context:
19+
output_file(empty_dir, "requirements.txt", "pip")
20+
21+
assert "requirements.txt file is required" in str(context.value)
22+
23+
24+
def test_detect_environment_requires_requirements_txt():
25+
"""Test that detect_environment raises an exception when requirements.txt is missing"""
26+
with tempfile.TemporaryDirectory() as empty_dir:
27+
with pytest.raises(EnvironmentException) as context:
28+
detect_environment(empty_dir, force_generate=False)
29+
30+
assert "requirements.txt file is required" in str(context.value)
31+
32+
33+
def test_detect_environment_with_force_generate():
34+
"""Test that detect_environment still works with force_generate=True"""
35+
with tempfile.TemporaryDirectory() as empty_dir:
36+
with mock.patch('rsconnect.subprocesses.inspect_environment.pip_freeze') as mock_pip_freeze:
37+
mock_pip_freeze.return_value = {
38+
"filename": "requirements.txt",
39+
"contents": "numpy\npandas",
40+
"source": "pip_freeze",
41+
"package_manager": "pip",
42+
}
43+
# This should not raise an exception
44+
environment = detect_environment(empty_dir, force_generate=True)
45+
assert environment.filename == "requirements.txt"
46+
assert "numpy" in environment.contents
47+
assert "pandas" in environment.contents

0 commit comments

Comments
 (0)