Repository navigation
Conversation
d7219fd to
2600591
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
2600591 to
b2a73ec
Compare
b2a73ec to
f3cf727
Compare
f3cf727 to
f241e8e
Compare
f241e8e to
e255248
Compare
e255248 to
c907cfe
Compare
11b0439 to
45d3cda
Compare
8d51c8c to
c7c2f0f
Compare
[Medium impact] The PR appears safe to merge; no outstanding findings remain. SummaryAdds a YAML launcher for proxy applications.
The previous documentation finding is fixed. No new actionable issues were found. Runtime tests were not run.
|
| `validate` checks the declaration, directory/config-file availability, upstream | ||
| URL rules and route composition. It does not read Rails contents, import their | ||
| actions, start engines, contact providers or prove checker compatibility. |
There was a problem hiding this comment.
validate only checks that each Rails directory exists, not that config files are available. The new tests explicitly accept an empty directory. Remove “config-file availability” from this description so operators do not rely on a guarantee the command does not provide.
| `validate` checks the declaration, directory/config-file availability, upstream | |
| URL rules and route composition. It does not read Rails contents, import their | |
| actions, start engines, contact providers or prove checker compatibility. | |
| `validate` checks the declaration, directory availability, upstream URL rules | |
| and route composition. It does not check config-file availability, read Rails | |
| contents, import their actions, start engines, contact providers or prove checker | |
| compatibility. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/server/experimental/application.md
Line: 77-79
Comment:
**Validation promises too much**
`validate` only checks that each Rails directory exists, not that config files are available. The new tests explicitly accept an empty directory. Remove “config-file availability” from this description so operators do not rely on a guarantee the command does not provide.
```suggestion
`validate` checks the declaration, directory availability, upstream URL rules
and route composition. It does not check config-file availability, read Rails
contents, import their actions, start engines, contact providers or prove checker
compatibility.
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
c7c2f0f to
0a725ca
Compare
0a725ca to
076e078
Compare
Let operators validate, inspect and serve the existing typed composition without Python glue. Keep the CLI inside the server so independent packaging does not depend on the library executable. Derive manifest validation and editor schema from the Python configuration, require fixed Rails bindings, bound YAML parsing, reject indirection and sanitize diagnostics. Inspect actual directional profiles without exposing unfinished variant selection. Reuse the existing lifespan and provide separate proxy/Rails example files. Manifest and CLI regressions include real-core partial catalogue rollback. Core cancellation and incomplete-cleanup prerequisites remain separate; the known queue cleanup retry regression still fails. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Adapt the preserved local-upstream harness to the server-owned module CLI and existing application assembly. Check native and LiteLLM Chat and Responses in buffered and streaming modes, block input before dispatch, and withhold blocked output. Verify guarded and passthrough disconnects close upstream sockets, later requests still work, and launcher subprocesses terminate cleanly. Fixtures are synthetic and use no live provider credentials. These smoke tests do not establish resolution of the separate interrupted-cleanup prerequisites. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Keep the manifest and Rails example under the server's ownership so the planned standalone-package move can carry them together. Update guide commands and socket-test paths without changing runtime or configuration behavior. Validation: 71 manifest and localhost launcher tests passed; changed-file pre-commit passed. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Replace smoke-test markers with a local email-pattern input/output rule and use explicit 200/50 streaming windows. Explain provider-delta units, buffering latency, detection limits, and the inability to retract previously released prefixes. Exercise the shipped configuration over real sockets, including split addresses, partial and full blocked windows, and downstream disconnects. Leave temporary smoke tooling untouched. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Copy the complete bundled example into a deployment-owned directory and use the local proxy.yaml path for launcher commands. Keep the module entry point supported by this branch; the standalone executable belongs to the later packaging change. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Create an isolated Windows process group and send CTRL_BREAK_EVENT, which Uvicorn handles for graceful shutdown, instead of forcefully terminating the process. Keep the bounded shutdown wait, forced reap on failure and shutdown-complete assertion on every platform; scope the POSIX signal exit-code assertion to POSIX. Validation: all five real-socket launcher tests passed on Python 3.10 and 3.13; pre-commit passed. Windows execution requires CI confirmation. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Allow 30 seconds for server draining and the existing Rails and HTTP cleanup budgets. Reap the child before reporting a timeout and include its complete output. Require shutdown completion without cleanup-failure or cleanup-timeout warnings on every platform. Validation: all five real-socket launcher tests passed on Python 3.10 and 3.13; pre-commit passed. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Send an explicit TCP write EOF and drain the peer's remaining response before closing the downstream socket. Keep the live ASGI disconnect, upstream closure and continued-serving assertions while avoiding a reset that can interrupt Windows Proactor socket cleanup before server detachment. Preserve normal cleanup budgets for healthy closers and shorten them only for intentionally hanging test doubles. Validation: 98 launcher and streaming HTTP tests passed on Python 3.10 and 3.13; pre-commit passed. Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
076e078 to
a85214f
Compare
Description
Add bounded YAML loading and server-owned
validate,inspect,schemaandservecommands. Include a local Rails example and offline socket tests; reuse the typed configuration and existing lifespan.Stack: #2466 → #2467 → #2468. Package separation follows separately; API-docs prototype excluded.
Summary by CodeRabbit
New Features
Documentation
Tests