Fix NAT FastAPI front end with Stdio-MCP server fails to initialize#788
Conversation
WalkthroughAdds a platform-specific event loop selection in the non-Gunicorn FastAPI startup: imports Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant Plugin as FastAPIFrontEndPlugin.run()
participant Sys as sys.platform
participant Uvicorn as uvicorn.run()
Caller->>Plugin: start()
alt under Gunicorn
Plugin-->>Caller: defer to Gunicorn flow
else direct startup
Plugin->>Sys: read platform
alt platform == "darwin" or startswith("linux")
Plugin-->>Plugin: event_loop_policy = "asyncio"
else
Plugin-->>Plugin: event_loop_policy = "auto"
end
Plugin->>Uvicorn: run(app, loop=event_loop_policy, reload_excludes=...)
Uvicorn-->>Caller: server running
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (2)
157-160: Fix ruff RUF003: replace curly apostrophes and prefer “macOS”.Curly quotes in comments trigger RUF003 and “MacOS” casing is inconsistent. Apply this comment-only patch:
- # By default, Uvicorn uses "auto" event loop policy, which prefers `uvloop` if installed. However, - # uvloop’s event loop policy for MacOS with Python 3.12 doesn’t provide a child watcher (which is needed - # for MCP server), so setting loop="asyncio" forces Uvicorn to use the standard event loop, which - # includes child-watcher support. + # By default, Uvicorn uses "auto" event loop policy, which prefers `uvloop` if installed. However, + # uvloop's event loop policy on macOS with Python 3.12 does not provide a child watcher (which is needed + # for the MCP server), so setting loop="asyncio" forces Uvicorn to use the standard event loop, which + # includes child-watcher support.
161-167: Optional: gate the macOS override to Python 3.12+ to minimize behavior change.If the failure is specific to 3.12 on macOS, this keeps uvloop performance for earlier versions.
- if sys.platform == "darwin": - # For MacOS + if sys.platform == "darwin" and sys.version_info >= (3, 12): + # For macOS + Python 3.12+ event_loop_policy = "asyncio" else: - # For non-MacOS platforms + # For non-macOS platforms or earlier Pythons event_loop_policy = "auto"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
🪛 Ruff (0.12.2)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
158-158: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
158-158: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
🔇 Additional comments (2)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (2)
19-19: Import for platform check is appropriate.
sysis needed for the macOS platform guard; no issues.
174-175: Confirmed — no change required: uvicorn.run accepts loop= with the pinned uvicorn (~=0.32.0).Manifest declares uvicorn[standard]~=0.32.0 and Uvicorn's programmatic API/Config accept a
loopkwarg (same options as the CLI--loop); docs/examples also showloopused in worker CONFIG_KWARGS. (uvicorn.org)
zhongxuanwang-nv
changed the title
Signed-off-by: Daniel Wang <daniewang@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (1)
200-206: Also enforce asyncio loop on macOS in the Gunicorn path.
UvicornWorkerwill default to uvloop if present; the same failure can reappear on macOS. SetUVICORN_LOOP=asynciofor Darwin before starting Gunicorn.Apply:
options = { "bind": f"{self.front_end_config.host}:{self.front_end_config.port}", "workers": self.front_end_config.workers, "worker_class": "uvicorn.workers.UvicornWorker", } + if sys.platform == "darwin": + # Ensure UvicornWorker uses asyncio loop on macOS as well. + os.environ.setdefault("UVICORN_LOOP", "asyncio") + StandaloneApplication(app, options=options).run()
🧹 Nitpick comments (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (1)
157-160: Fix RUF003: replace curly apostrophes in comments.Ruff flags RIGHT SINGLE QUOTATION MARK. Use ASCII
'.Apply:
- # uvloop’s event loop policy for macOS doesn’t provide a child watcher (which is needed for MCP server), + # uvloop's event loop policy for macOS doesn't provide a child watcher (which is needed for the MCP server),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
🪛 Ruff (0.12.2)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
158-158: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
158-158: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
🔇 Additional comments (3)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (3)
19-19: Import for platform check — LGTM.
sysis required for the macOS gate below.
161-167: macOS loop override is correct and well-scoped.Forcing
loop="asyncio"on Darwin addresses the child-watcher gap; elsewhere retainsauto.
174-175: Confirmed — passing loop into uvicorn.run is supported for the pinned version.pyproject.toml pins uvicorn[standard]~=0.32.0 (pyproject.toml:47); uvicorn.run forwards kwargs to Config and Config exposes a "loop" option, so passing loop=event_loop_policy is supported. (github.com)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (1)
162-168: Extend fix beyond macOS; add safe override.Ubuntu/Linux can hit the same NotImplementedError with stdio. Provide an env override and default to asyncio on darwin and linux.
Apply this diff:
- if sys.platform == "darwin": - # For macOS - event_loop_policy = "asyncio" - else: - # For non-macOS platforms - event_loop_policy = "auto" + # Allow manual override for debugging or platform quirks. + loop_override = os.environ.get("UVICORN_LOOP") or os.environ.get("NAT_FORCE_ASYNCIO_LOOP") + if loop_override in {"asyncio", "uvloop", "auto"}: + event_loop_policy = loop_override + elif sys.platform in {"darwin", "linux"}: + # macOS and some Linux environments need asyncio for subprocess/stdio support. + event_loop_policy = "asyncio" + else: + event_loop_policy = "auto"Please re-test:
- macOS + stdio MCP: expect startup OK.
- Ubuntu + stdio MCP: expect startup OK (no NotImplementedError).
- Non-stdio configs: verify default performance remains acceptable.
🧹 Nitpick comments (2)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (2)
158-161: Fix RUF003: replace curly quotes in comments.Ruff flags RIGHT SINGLE QUOTATION MARK (’). Use ASCII
'.Apply this diff:
- # By default, Uvicorn uses "auto" event loop policy, which prefers `uvloop` if installed. However, - # uvloop’s event loop policy for macOS doesn’t provide a child watcher (which is needed for MCP server), - # so setting loop="asyncio" forces Uvicorn to use the standard event loop, which includes child-watcher - # support. + # By default, Uvicorn uses "auto" event loop policy, which prefers `uvloop` if installed. However, + # uvloop's event loop policy for macOS doesn't provide a child watcher (needed by MCP stdio), + # so setting loop="asyncio" forces Uvicorn to use the standard event loop, which includes child-watcher + # support.
162-168: Name nit: avoid “policy” confusion.This is a loop mode, not an asyncio event loop policy.
Apply this diff:
- event_loop_policy = ... + loop_mode = ... @@ - reload_excludes=reload_excludes, - loop=event_loop_policy) + reload_excludes=reload_excludes, + loop=loop_mode)Also applies to: 175-176
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
🪛 Ruff (0.12.2)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
159-159: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
159-159: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
🔇 Additional comments (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (1)
19-19: Import looks good.
sysis needed for platform detection.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (1)
201-205: Gunicorn path still defaults to uvloop — stdio will still fail on macOS/Linux.When
use_gunicorn=True,UvicornWorkerwill ignore the non‑Gunicorn loop selection and default to uvloop. Subclass the worker and inject the same loop.Apply this diff:
- "worker_class": "uvicorn.workers.UvicornWorker", + "worker_class": "nat.front_ends.fastapi.fastapi_front_end_plugin:UvicornWorkerWithLoop",Add the subclass (outside this block):
from uvicorn.workers import UvicornWorker class UvicornWorkerWithLoop(UvicornWorker): CONFIG_KWARGS = {"loop": _select_uvicorn_loop()}Verify with stdio +
use_gunicorn: trueon macOS/Linux:#!/bin/bash # Find gunicorn usage and worker class settings rg -nP 'use_gunicorn\s*:\s*true' -g '**/*.{yml,yaml}' || true rg -n 'worker_class.*UvicornWorker' || true
🧹 Nitpick comments (6)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (6)
158-176: Unify loop selection; pass it via helper; fix comments; use it in uvicorn.run.
- Current comments say “For macOS” but the condition includes Linux.
- Prefer a single
_select_uvicorn_loop()to centralize the rule and allow an env override.Apply this diff within the shown range:
- # By default, Uvicorn uses "auto" event loop policy, which prefers `uvloop` if installed. However, - # uvloop’s event loop policy for macOS doesn’t provide a child watcher (which is needed for MCP server), - # so setting loop="asyncio" forces Uvicorn to use the standard event loop, which includes child-watcher - # support. - if sys.platform == "darwin" or sys.platform.startswith("linux"): - # For macOS - event_loop_policy = "asyncio" - else: - # For non-macOS platforms - event_loop_policy = "auto" + # Select Uvicorn loop: force "asyncio" on macOS/Linux for MCP stdio (child watcher), + # otherwise fall back to "auto". Allow override via environment. + loop_mode = _select_uvicorn_loop() @@ - reload_excludes=reload_excludes, - loop=event_loop_policy) + reload_excludes=reload_excludes, + loop=loop_mode)Add the following helper (outside this block, e.g., near other module-level defs):
def _select_uvicorn_loop() -> str: override = os.environ.get("UVICORN_LOOP") or os.environ.get("NAT_FORCE_UVICORN_LOOP") if override in {"asyncio", "uvloop", "auto"}: return override if sys.platform == "darwin" or sys.platform.startswith("linux"): return "asyncio" return "auto"
159-161: Fix RUF003: ambiguous right single quote in comments.Replace curly ’ with ASCII ' to satisfy ruff.
Minimal change (if not adopting the larger refactor above):
- # uvloop’s event loop policy for macOS doesn’t provide a child watcher (which is needed for MCP server), + # uvloop's event loop policy for macOS doesn't provide a child watcher (which is needed for MCP server),
93-94: Add return type and a brief docstring torun(public API).Meets repo policy (type hints + Google-style docstrings).
- async def run(self): + async def run(self) -> None: + """Start the NAT FastAPI front end. + + On macOS/Linux, forces the asyncio loop for MCP stdio (child watcher support), + then starts Uvicorn or Gunicorn based on configuration and wires Dask cleanup. + """
37-41: Docstrings and return annotations for public methods.Add
-> Noneand concise docstrings to satisfy coding guidelines and avoid pyright noise.class FastApiFrontEndPlugin(DaskClientMixin, FrontEndBase[FastApiFrontEndConfig]): + """FastAPI front end plugin for NAT.""" - def __init__(self, full_config: "Config"): + def __init__(self, full_config: "Config") -> None: + """Initialize the front end with the resolved `Config`.""" super().__init__(full_config) - async def _periodic_cleanup(scheduler_address: str, + async def _periodic_cleanup(scheduler_address: str, db_url: str, sleep_time_sec: int = 300, log_level: int = logging.INFO): + """Periodically remove expired jobs from the job store.""" @@ - async def _submit_cleanup_task(self, scheduler_address: str, db_url: str): + async def _submit_cleanup_task(self, scheduler_address: str, db_url: str) -> None: """Submit a cleanup task to the cluster to remove the job after expiry."""Also applies to: 62-68, 82-92
133-138: Fix typo in comment.- # Set environment variabls such that the worker subprocesses will know how to connect to dask and to - # the database + # Set environment variables so worker subprocesses know how to connect to Dask and the database
77-80: Avoid bareexcept:; catchExceptionexplicitly.Keeps behavior, removes E722 suppression.
- except: # noqa: E722 - logger.exception("Error during job cleanup") + except Exception: + logger.exception("Error during job cleanup")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
🪛 Ruff (0.12.2)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py
159-159: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
159-159: Comment contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF003)
🔇 Additional comments (1)
src/nat/front_ends/fastapi/fastapi_front_end_plugin.py (1)
19-19: LGTM: needed import.
## Description <!-- Note: The pull request title will be included in the CHANGELOG. --> <!-- Provide a standalone description of changes in this PR. --> <!-- Reference any issues closed by this PR with "closes #1234". All PRs should have an issue they close--> This is a follow-up PR of [PR-788](#788). This PR will try to cleanly remove the `use_uvloop` field from the general section of the config files and deprecate that field to not create any breaking changes. ## By Submitting this PR I confirm: - I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing.md). - We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license. - Any contribution which contains commits that are not Signed-Off will not be accepted. - When the PR is ready for review, new or existing tests cover these changes. - When the PR is ready for review, the documentation is up to date with these changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Refactor - Deprecated the manual use_uvloop setting; event-loop selection is now auto-determined per platform. - Documentation - Updated docs and examples to remove explicit use_uvloop examples and add deprecation guidance. - Chores - Removed use_uvloop entries from sample configs and templates across examples and guides. - Tests - Cleansed test config data to remove explicit use_uvloop references. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Daniel Wang <daniewang@nvidia.com> Signed-off-by: Zhongxuan (Daniel) Wang <zxwang2004@gmail.com> Co-authored-by: Will Killian <2007799+willkill07@users.noreply.github.com>
3 participants
Description
Closes AIQ-1853
In the past, when you do
You will get something like
This is because when you start server with
uvicorn.run("nat.front_ends.fastapi.main:get_app",...,Uvicornby default usesloop="auto", which prefersuvloopif it’s installed.However, on macOS, uvloop’s event loop policy doesn’t provide a child watcher, so calls that spawn subprocesses (used by MCP stdio via asyncio) call
asyncio.events.get_child_watcher()and raise NotImplementedError.Therefore, we can fix it by setting loop="asyncio", which forces Uvicorn to use the standard asyncio event loop (which includes child-watcher support) when the system is MacOS. That enables asyncio.create_subprocess_exec to work, so the stdio MCP client can launch its subprocess cleanly.
Follow up, why uvloop’s policy doesn’t provide a child watcher?
uvloopis a high‑performance event loop for Python’s asyncio, implemented on top oflibuv(the C library used by Node.js). It’s a drop‑in replacement for the default asyncio loop and typically yields faster networking and scheduling.uvloop’s loop policy design relies on libuv’s own process handling rather than Python’s legacy child‑watcher abstraction.When code (e.g., anyio/asyncio paths) calls asyncio.get_event_loop_policy().get_child_watcher() under uvloop, the policy raises NotImplementedError because that API isn’t supported for MacOS.
By Submitting this PR I confirm:
Summary by CodeRabbit
Bug Fixes
Chores