feat(cli): add server workers option#1851
feat(cli): add server workers option#1851adityasingh2400 wants to merge 8 commits intoNVIDIA-NeMo:developfrom
Conversation
Documentation preview |
Greptile SummaryThis PR adds a
|
| Filename | Overview |
|---|---|
| nemoguardrails/cli/init.py | Adds --workers option; propagates all CLI flags via env vars before spawning workers; guards --prefix and --auto-reload against multi-worker mode; uses import-string + factory for multi-worker uvicorn invocation. |
| nemoguardrails/server/api.py | Reads new env vars at module-level so each worker process picks up configuration on fresh import; adds create_app() factory for uvicorn's factory mode; minor inconsistency between disable_chat_ui (env var read in __init__) and auto_reload (module-level override). |
| tests/cli/test_cli_main.py | Adds tests for env var propagation, workers factory path, and prefix/auto-reload rejection; however, the pre-existing test_server_with_disable_chat_ui and test_server_with_auto_reload tests are now broken by the new os.fspath call when the repo root has no config/ directory. |
| docs/reference/cli/index.md | Documents the new --workers option in the server command reference table; no issues. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["nemoguardrails server --workers N ..."] --> B{workers > 1?}
B -- No --> C["Set env vars\nNEMO_GUARDRAILS_CONFIG_PATH / VERBOSE\nDISABLE_CHAT_UI / AUTO_RELOAD\nDEFAULT_CONFIG_ID"]
B -- Yes --> D{prefix set?}
D -- Yes --> E["Exit 1: prefix not supported"]
D -- No --> F{auto_reload set?}
F -- Yes --> G["Exit 1: auto-reload not supported"]
F -- No --> C
C --> H{workers > 1?}
H -- No --> I["uvicorn.run(server_app, ...)"]
H -- Yes --> J["uvicorn.run('...api:create_app', factory=True, workers=N)"]
J --> K["Worker Process 1..N\nimport nemoguardrails.server.api\n(reads env vars at module level)"]
K --> L["create_app() returns configured app singleton"]
L --> M["Uvicorn serves requests"]
I --> M
Comments Outside Diff (1)
-
tests/cli/test_cli_main.py, line 195-207 (link)Existing tests break on CI —
os.fspathraisesTypeErroron mockedapi.appThe PR's new code path unconditionally calls
os.fspath(rails_config_path)before setting the env var. When neither--configis passed nor a localconfig/directory exists (confirmed absent at the repo root), the code falls through torails_config_path = api.app.rails_config_path, which is aMagicMockattribute in these two tests.os.fspathrejects anything that isn'tstr,bytes, or a properos.PathLike; it calls__fspath__()on the mock, gets back anotherMagicMock, and raisesTypeError. The invocation exits with code 1, causingassert result.exit_code == 0to fail. The three newly-added tests avoid this exactly by patchingos.getcwdandos.path.exists— the same treatment is needed here.Prompt To Fix With AI
This is a comment left during a code review. Path: tests/cli/test_cli_main.py Line: 195-207 Comment: **Existing tests break on CI — `os.fspath` raises `TypeError` on mocked `api.app`** The PR's new code path unconditionally calls `os.fspath(rails_config_path)` before setting the env var. When neither `--config` is passed nor a local `config/` directory exists (confirmed absent at the repo root), the code falls through to `rails_config_path = api.app.rails_config_path`, which is a `MagicMock` attribute in these two tests. `os.fspath` rejects anything that isn't `str`, `bytes`, or a proper `os.PathLike`; it calls `__fspath__()` on the mock, gets back another `MagicMock`, and raises `TypeError`. The invocation exits with code 1, causing `assert result.exit_code == 0` to fail. The three newly-added tests avoid this exactly by patching `os.getcwd` and `os.path.exists` — the same treatment is needed here. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
tests/cli/test_cli_main.py:195-207
**Existing tests break on CI — `os.fspath` raises `TypeError` on mocked `api.app`**
The PR's new code path unconditionally calls `os.fspath(rails_config_path)` before setting the env var. When neither `--config` is passed nor a local `config/` directory exists (confirmed absent at the repo root), the code falls through to `rails_config_path = api.app.rails_config_path`, which is a `MagicMock` attribute in these two tests. `os.fspath` rejects anything that isn't `str`, `bytes`, or a proper `os.PathLike`; it calls `__fspath__()` on the mock, gets back another `MagicMock`, and raises `TypeError`. The invocation exits with code 1, causing `assert result.exit_code == 0` to fail. The three newly-added tests avoid this exactly by patching `os.getcwd` and `os.path.exists` — the same treatment is needed here.
### Issue 2 of 3
tests/cli/test_cli_main.py:221-228
`test_server_api_reads_worker_env_on_import` seeds the subprocess env with `**os.environ`, so any `NEMO_GUARDRAILS_*` variable set in the CI runner bleeds into the subprocess alongside the intended overrides. Starting from a minimal, known-clean env makes the test hermetic.
```suggestion
def test_server_api_reads_worker_env_on_import(self):
env = {
"NEMO_GUARDRAILS_CONFIG_PATH": "/worker/config",
"NEMO_GUARDRAILS_DISABLE_CHAT_UI": "true",
"NEMO_GUARDRAILS_AUTO_RELOAD": "true",
"NEMO_GUARDRAILS_DEFAULT_CONFIG_ID": "worker_config",
}
```
### Issue 3 of 3
nemoguardrails/server/api.py:73-74
`auto_reload` is initialised to `False` inside `GuardrailsApp.__init__` then overridden by a module-level statement that reads the env var, while `disable_chat_ui` reads the env var directly in `__init__`. Aligning `auto_reload` to also read from the env var in `__init__` makes the initialisation pattern uniform.
```suggestion
self.disable_chat_ui: bool = os.getenv("NEMO_GUARDRAILS_DISABLE_CHAT_UI", "false").lower() == "true"
self.auto_reload: bool = os.getenv("NEMO_GUARDRAILS_AUTO_RELOAD", "false").lower() == "true"
```
Reviews (7): Last reviewed commit: "test(server): isolate verbose worker env..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| api.set_default_config_id(default_config_id) # Call function | ||
|
|
||
| uvicorn.run(server_app, port=port, log_level="info", host="0.0.0.0") | ||
| uvicorn.run(server_app, port=port, log_level="info", host="0.0.0.0", workers=workers) |
There was a problem hiding this comment.
workers > 1 silently exits when app is passed as an instance
Uvicorn requires an import string (e.g. "nemoguardrails.server.api:app") — not an application object — when workers is set to anything greater than 1. Passing the live server_app object with workers=2 causes uvicorn to print "You must pass the application as an import string to enable 'reload' or 'workers'" and then exit with SystemError 1. This means every invocation of --workers N where N > 1 is silently broken.
The fix requires either (a) passing the app as an import string when workers > 1 (which complicates the current per-process attribute-patching approach used for rails_config_path, disable_chat_ui, etc.), or (b) raising an explicit error when workers > 1 with a clear message about this constraint until a proper solution is implemented.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/cli/__init__.py
Line: 204
Comment:
**`workers > 1` silently exits when app is passed as an instance**
Uvicorn requires an import string (e.g. `"nemoguardrails.server.api:app"`) — not an application object — when `workers` is set to anything greater than 1. Passing the live `server_app` object with `workers=2` causes uvicorn to print `"You must pass the application as an import string to enable 'reload' or 'workers'"` and then exit with `SystemError 1`. This means every invocation of `--workers N` where N > 1 is silently broken.
The fix requires either (a) passing the app as an import string when `workers > 1` (which complicates the current per-process attribute-patching approach used for `rails_config_path`, `disable_chat_ui`, etc.), or (b) raising an explicit error when `workers > 1` with a clear message about this constraint until a proper solution is implemented.
How can I resolve this? If you propose a fix, please make it concise.
📝 WalkthroughWalkthroughA ChangesMulti-Worker Server Support
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nemoguardrails/cli/__init__.py`:
- Line 204: The current call uvicorn.run(server_app, ...) passes a live app
instance so --workers>1 fails and the per-process mutations (rails_config_path,
disable_chat_ui, auto_reload, default_config_id set between lines 173-202) don’t
propagate to workers; change to run Uvicorn with an import string or factory so
workers spawn correctly and receive configuration: expose a factory like
create_app(...) or export the ASGI app as a module-level name (e.g.,
"nemoguardrails.cli.__init__:server_app" or better
"nemoguardrails.cli.__init__:create_app") and call uvicorn.run with that import
string when workers>1, and move the mutating logic (rails_config_path,
disable_chat_ui, auto_reload, default_config_id) into the app factory or into
startup hooks/environment variables so each worker initializes with the intended
config.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f065d2bf-33da-4e7d-9f2d-b9bcf077d8cf
📒 Files selected for processing (2)
docs/reference/cli/index.mdnemoguardrails/cli/__init__.py
| api.set_default_config_id(default_config_id) # Call function | ||
|
|
||
| uvicorn.run(server_app, port=port, log_level="info", host="0.0.0.0") | ||
| uvicorn.run(server_app, port=port, log_level="info", host="0.0.0.0", workers=workers) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
According to the latest Uvicorn documentation, can uvicorn.run(app_instance, workers=2) run correctly, or must multi-worker mode use an import string / app factory? Also confirm how worker process spawning affects parent-process app mutations.
💡 Result:
No, uvicorn.run(app_instance, workers=2) cannot run correctly in multi-worker mode. According to the latest Uvicorn documentation and deployment guides, when using workers > 1 (or reload=True), the application must be passed as an import string (e.g., "module:app") rather than a direct app instance. Passing an app instance triggers a warning and fails to spawn workers properly, as Uvicorn requires the import path to load fresh app instances in each worker process.[1][2][3][4][5][6] For app factories, use factory=True with an import string to a callable that returns the app (e.g., uvicorn.run("module:create_app", factory=True, workers=2)).[7][3] Worker process spawning uses multiprocessing (fork on Unix-like systems, spawn on Windows/macOS). The parent process acts as a manager and does not handle requests; each worker independently imports and instantiates the app from the string. Mutations to the app in the parent process (e.g., adding routes) do not affect workers, as they load their own copies after forking/spawning. Workers are isolated processes with copy-on-write memory (Unix) or fresh execution (spawn), so parent mutations post-spawn have no effect.[8][4][9][10][11]
Citations:
- 1: https://uvicorn.dev/settings/
- 2: https://uvicorn.dev/deployment/
- 3: https://uvicorn.dev/
- 4: https://bophin.com/article/deployment-uvicorn
- 5: https://openillumi.com/en/en-fastapi-poetry-uvicorn-reload-fix-pyproject-toml/
- 6: Run uvicorn with multiple workers on Windows - WinError 5 Kludex/uvicorn#484
- 7: https://uvicorn.run/
- 8: [question] Why is running multiple workers with an existing app object no longer allowed? Kludex/uvicorn#540
- 9: uvicorn.run with reload or workers>1 does not work when called from multiprocessing with spawn Kludex/uvicorn#2679
- 10: Cannot share objects from main process to Uvicorn workers Kludex/uvicorn#1019
- 11: https://fastapi.tiangolo.com/deployment/server-workers/
🏁 Script executed:
# First, let's look at the context around line 204
cat -n nemoguardrails/cli/__init__.py | sed -n '190,220p'Repository: NVIDIA-NeMo/Guardrails
Length of output: 1314
🏁 Script executed:
# Get the function signature and more context around line 204
cat -n nemoguardrails/cli/__init__.py | sed -n '150,210p'Repository: NVIDIA-NeMo/Guardrails
Length of output: 2376
🏁 Script executed:
# Get the function signature starting from `@app.command`() decorator
cat -n nemoguardrails/cli/__init__.py | sed -n '100,160p'Repository: NVIDIA-NeMo/Guardrails
Length of output: 2406
--workers parameter is broken with multi-worker mode due to app instance limitation.
At line 204, uvicorn.run() is passed a live app instance (server_app). When workers > 1, Uvicorn cannot spawn multiple workers from an app instance—it requires an import string (e.g., "module:app") or a factory function. Additionally, critical app mutations (lines 173-202) including rails_config_path, disable_chat_ui, auto_reload, and default_config_id are applied in the parent process and will not propagate to worker processes, as each worker loads a fresh copy of the app. This renders the --workers flag non-functional for any value greater than 1, with no error message to alert users.
🧰 Tools
🪛 Ruff (0.15.12)
[error] 204-204: Possible binding to all interfaces
(S104)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nemoguardrails/cli/__init__.py` at line 204, The current call
uvicorn.run(server_app, ...) passes a live app instance so --workers>1 fails and
the per-process mutations (rails_config_path, disable_chat_ui, auto_reload,
default_config_id set between lines 173-202) don’t propagate to workers; change
to run Uvicorn with an import string or factory so workers spawn correctly and
receive configuration: expose a factory like create_app(...) or export the ASGI
app as a module-level name (e.g., "nemoguardrails.cli.__init__:server_app" or
better "nemoguardrails.cli.__init__:create_app") and call uvicorn.run with that
import string when workers>1, and move the mutating logic (rails_config_path,
disable_chat_ui, auto_reload, default_config_id) into the app factory or into
startup hooks/environment variables so each worker initializes with the intended
config.
| if workers > 1: | ||
| if prefix: | ||
| typer.secho( | ||
| "The --prefix option is not supported with --workers > 1.", | ||
| fg=typer.colors.RED, | ||
| ) | ||
| raise typer.Exit(1) |
There was a problem hiding this comment.
--auto-reload and --workers > 1 are combined, each uvicorn worker process inherits NEMO_GUARDRAILS_AUTO_RELOAD=true and independently enters the lifespan's start_auto_reload_monitoring branch, spawning N concurrent file watchers on the same directory. This causes redundant reload signals and potential race conditions. The PR already guards --prefix against workers > 1; the same guard is needed for --auto-reload.
| if workers > 1: | |
| if prefix: | |
| typer.secho( | |
| "The --prefix option is not supported with --workers > 1.", | |
| fg=typer.colors.RED, | |
| ) | |
| raise typer.Exit(1) | |
| if workers > 1: | |
| if prefix: | |
| typer.secho( | |
| "The --prefix option is not supported with --workers > 1.", | |
| fg=typer.colors.RED, | |
| ) | |
| raise typer.Exit(1) | |
| if auto_reload: | |
| typer.secho( | |
| "The --auto-reload option is not supported with --workers > 1.", | |
| fg=typer.colors.RED, | |
| ) | |
| raise typer.Exit(1) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/cli/__init__.py
Line: 210-216
Comment:
When `--auto-reload` and `--workers > 1` are combined, each uvicorn worker process inherits `NEMO_GUARDRAILS_AUTO_RELOAD=true` and independently enters the lifespan's `start_auto_reload_monitoring` branch, spawning N concurrent file watchers on the same directory. This causes redundant reload signals and potential race conditions. The PR already guards `--prefix` against `workers > 1`; the same guard is needed for `--auto-reload`.
```suggestion
if workers > 1:
if prefix:
typer.secho(
"The --prefix option is not supported with --workers > 1.",
fg=typer.colors.RED,
)
raise typer.Exit(1)
if auto_reload:
typer.secho(
"The --auto-reload option is not supported with --workers > 1.",
fg=typer.colors.RED,
)
raise typer.Exit(1)
```
How can I resolve this? If you propose a fix, please make it concise.
| @patch("uvicorn.run") | ||
| @patch("nemoguardrails.server.api.app") | ||
| def test_server_rejects_prefix_with_workers(self, mock_app, mock_uvicorn): | ||
| result = runner.invoke(app, ["server", "--workers=2", "--prefix=/api/v1"]) | ||
|
|
||
| assert result.exit_code == 1 | ||
| assert "The --prefix option is not supported with --workers > 1." in result.output | ||
| mock_uvicorn.assert_not_called() | ||
|
|
||
| @patch("uvicorn.run") | ||
| @patch("nemoguardrails.server.api.app") | ||
| def test_server_rejects_auto_reload_with_workers(self, mock_app, mock_uvicorn): | ||
| result = runner.invoke(app, ["server", "--workers=2", "--auto-reload"]) | ||
|
|
||
| assert result.exit_code == 1 | ||
| assert "The --auto-reload option is not supported with --workers > 1." in result.output | ||
| mock_uvicorn.assert_not_called() |
There was a problem hiding this comment.
os.getcwd() / os.path.exists() and don't guard the environment with @patch.dict. When there is no config/ directory in the working directory, rails_config_path falls back to api.app.rails_config_path (a MagicMock), which means os.fspath returns "" and NEMO_GUARDRAILS_CONFIG_PATH is set to an empty string in the real os.environ. That leaked value persists into subsequent tests in the same process and could silently affect any test that reads NEMO_GUARDRAILS_CONFIG_PATH without clearing it first. Adding the same isolation decoration used by the other workers test fixes both problems.
| @patch("uvicorn.run") | |
| @patch("nemoguardrails.server.api.app") | |
| def test_server_rejects_prefix_with_workers(self, mock_app, mock_uvicorn): | |
| result = runner.invoke(app, ["server", "--workers=2", "--prefix=/api/v1"]) | |
| assert result.exit_code == 1 | |
| assert "The --prefix option is not supported with --workers > 1." in result.output | |
| mock_uvicorn.assert_not_called() | |
| @patch("uvicorn.run") | |
| @patch("nemoguardrails.server.api.app") | |
| def test_server_rejects_auto_reload_with_workers(self, mock_app, mock_uvicorn): | |
| result = runner.invoke(app, ["server", "--workers=2", "--auto-reload"]) | |
| assert result.exit_code == 1 | |
| assert "The --auto-reload option is not supported with --workers > 1." in result.output | |
| mock_uvicorn.assert_not_called() | |
| @patch.dict(os.environ, {}, clear=True) | |
| @patch("uvicorn.run") | |
| @patch("nemoguardrails.server.api.app") | |
| @patch("os.path.exists") | |
| @patch("os.getcwd") | |
| def test_server_rejects_prefix_with_workers( | |
| self, mock_getcwd, mock_exists, mock_app, mock_uvicorn | |
| ): | |
| mock_getcwd.return_value = "/current/dir" | |
| mock_exists.return_value = True | |
| result = runner.invoke(app, ["server", "--workers=2", "--prefix=/api/v1"]) | |
| assert result.exit_code == 1 | |
| assert "The --prefix option is not supported with --workers > 1." in result.output | |
| mock_uvicorn.assert_not_called() | |
| @patch.dict(os.environ, {}, clear=True) | |
| @patch("uvicorn.run") | |
| @patch("nemoguardrails.server.api.app") | |
| @patch("os.path.exists") | |
| @patch("os.getcwd") | |
| def test_server_rejects_auto_reload_with_workers( | |
| self, mock_getcwd, mock_exists, mock_app, mock_uvicorn | |
| ): | |
| mock_getcwd.return_value = "/current/dir" | |
| mock_exists.return_value = True | |
| result = runner.invoke(app, ["server", "--workers=2", "--auto-reload"]) | |
| assert result.exit_code == 1 | |
| assert "The --auto-reload option is not supported with --workers > 1." in result.output | |
| mock_uvicorn.assert_not_called() |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/cli/test_cli_main.py
Line: 257-273
Comment:
The two reject tests don't mock `os.getcwd()` / `os.path.exists()` and don't guard the environment with `@patch.dict`. When there is no `config/` directory in the working directory, `rails_config_path` falls back to `api.app.rails_config_path` (a `MagicMock`), which means `os.fspath` returns `""` and `NEMO_GUARDRAILS_CONFIG_PATH` is set to an empty string in the real `os.environ`. That leaked value persists into subsequent tests in the same process and could silently affect any test that reads `NEMO_GUARDRAILS_CONFIG_PATH` without clearing it first. Adding the same isolation decoration used by the other workers test fixes both problems.
```suggestion
@patch.dict(os.environ, {}, clear=True)
@patch("uvicorn.run")
@patch("nemoguardrails.server.api.app")
@patch("os.path.exists")
@patch("os.getcwd")
def test_server_rejects_prefix_with_workers(
self, mock_getcwd, mock_exists, mock_app, mock_uvicorn
):
mock_getcwd.return_value = "/current/dir"
mock_exists.return_value = True
result = runner.invoke(app, ["server", "--workers=2", "--prefix=/api/v1"])
assert result.exit_code == 1
assert "The --prefix option is not supported with --workers > 1." in result.output
mock_uvicorn.assert_not_called()
@patch.dict(os.environ, {}, clear=True)
@patch("uvicorn.run")
@patch("nemoguardrails.server.api.app")
@patch("os.path.exists")
@patch("os.getcwd")
def test_server_rejects_auto_reload_with_workers(
self, mock_getcwd, mock_exists, mock_app, mock_uvicorn
):
mock_getcwd.return_value = "/current/dir"
mock_exists.return_value = True
result = runner.invoke(app, ["server", "--workers=2", "--auto-reload"])
assert result.exit_code == 1
assert "The --auto-reload option is not supported with --workers > 1." in result.output
mock_uvicorn.assert_not_called()
```
How can I resolve this? If you propose a fix, please make it concise.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| @patch.dict(os.environ, {}, clear=True) | ||
| @patch("uvicorn.run") | ||
| @patch("nemoguardrails.server.api.app") | ||
| def test_server_with_verbose_sets_worker_env(self, mock_app, mock_uvicorn): | ||
| result = runner.invoke(app, ["server", "--verbose"]) | ||
| assert result.exit_code == 0 | ||
| assert os.environ["NEMO_GUARDRAILS_VERBOSE"] == "true" |
There was a problem hiding this comment.
os.getcwd / os.path.exists mocks — test fails in CI
When neither --config is passed nor a config/ directory exists in the cwd, the CLI falls back to rails_config_path = api.app.rails_config_path (the patched MagicMock). The subsequent os.fspath(mock_app.rails_config_path) call requires __fspath__() to return a str or bytes; a MagicMock auto-attribute satisfies neither, so os.fspath raises TypeError. The repo root has no config/ directory, so os.path.exists returns False and the error path is always hit. result.exit_code will be 1 and the assert result.exit_code == 0 will fail. The three newly-added worker/reject tests correctly use @patch("os.getcwd") + @patch("os.path.exists") for exactly this reason — the same fix is needed here.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/cli/test_cli_main.py
Line: 209-215
Comment:
**Missing `os.getcwd` / `os.path.exists` mocks — test fails in CI**
When neither `--config` is passed nor a `config/` directory exists in the cwd, the CLI falls back to `rails_config_path = api.app.rails_config_path` (the patched MagicMock). The subsequent `os.fspath(mock_app.rails_config_path)` call requires `__fspath__()` to return a `str` or `bytes`; a MagicMock auto-attribute satisfies neither, so `os.fspath` raises `TypeError`. The repo root has no `config/` directory, so `os.path.exists` returns `False` and the error path is always hit. `result.exit_code` will be `1` and the `assert result.exit_code == 0` will fail. The three newly-added worker/reject tests correctly use `@patch("os.getcwd")` + `@patch("os.path.exists")` for exactly this reason — the same fix is needed here.
How can I resolve this? If you propose a fix, please make it concise.
Pouyanpi
force-pushed
the
develop
branch
4 times, most recently
from
a6be550 to
c69efe5
Compare
Add os.getcwd / os.path.exists mocks to test_server_with_verbose_sets_worker_env so it does not rely on MagicMock's __fspath__ fallback or the absence of a config/ directory in the working tree, matching the isolation pattern used by the other worker-related tests. Signed-off-by: Aditya Singh <adisin650@gmail.com>
|
Addressed greptile review comment on the missing |
1 participant
Summary
--workersoption tonemoguardrails server.uvicorn.run.Fixes #1746
Verification
python3 -m compileall nemoguardrails/cli/__init__.pyPR overlap check
1746; no existing open PR covers this issue.Summary by CodeRabbit
--workersCLI option to the server command for specifying the number of Uvicorn worker processes to run (default: 1, minimum: 1), enabling multi-process serving configurations.