feat(dev): add local model serving tool - #212
Conversation
da1d319 to
a3a8702
Compare
3f68c14 to
b57974c
Compare
981c1dc to
f8bd142
Compare
|
/nvskills-ci |
2 similar comments
|
/nvskills-ci |
|
/nvskills-ci |
|
/nvskills-ci |
fb09707 to
1dd931e
Compare
|
/nvskills-ci |
|
Post-rebase A100 dogfood completed on HEAD
The analyzer reproduced the seven previously verified measurement false positives (1 biographies, 6 legal). No leak-metric changes are included in this hoster PR. |
8093109 to
08b7332
Compare
|
/nvskills-ci |
08b7332 to
9d9536b
Compare
|
/nvskills-ci |
|
Added pinned GPT-OSS 120B, GPT-OSS 20B, and Qwen3 30B A3B Instruct profiles on rebased HEAD |
|
Medium hoster simplification is now on
The implementation began as an Engineer Arc candidate. Parent review rejected its over-aggressive test reduction, restored lifecycle-focused behavioral coverage, removed a stray compiler branch, and reran all validation before push. |
|
/nvskills-ci |
|
Added the local-model deployment docs and checked-in GPU container image in commit 0034b8f. Validation: full suite 1,226 passed; make check passed; strict docs build passed; Docker image build passed; packaged CLI and vLLM Factory compatibility doctor passed inside the image. |
|
/nvskills-ci |
|
Modern Python boundary refactor is now on HEAD
A Terra implementation and independent-review cycle found and resolved plugin-spec duplication, secret ownership, positional chunk tuples, and stale delegation seams. Primary-agent review then caught the empty-source-revision exit regression and the remaining manual HTTP ownership flag before push. Validation on the exact signed commit chain:
No bundled skill files changed, so no new |
|
Semantic cleanup is now on HEAD
A Terra reviewed-engineering Arc performed implementation, independent review, bounded fix-up, and confirmation. Review initially blocked on the PID-marker fallback and stale shipped instructions; both were fixed, and confirmation accepted the exact final tree with no findings. Validation on that tree:
|
|
/nvskills-ci |
|
Follow-up council warnings addressed in abf8a7e:
Validation:
|
|
Post-fix A100 dogfood passed on HEAD abf8a7e.
Run artifacts are retained locally under /tmp/anonymizer-abf8a7e-full-run-U7pB7p. |
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
5345b76 to
7b4e515
Compare
| ) | ||
|
|
||
| worker_count = min(len(chunks), MAX_CONCURRENT_POOLING_CALLS) | ||
| await asyncio.gather(*(worker() for _ in range(worker_count))) |
There was a problem hiding this comment.
Failed requests retain workers
When one chunk of a multi-chunk detector request receives a non-200 or malformed pooling response, asyncio.gather propagates the error without cancelling the sibling workers. The middleware then returns HTTP 400 while those workers continue submitting the failed request's remaining chunks through the shared semaphore, delaying unrelated detector requests.
There was a problem hiding this comment.
I agree this should use structured cancellation, such as TaskGroup, or explicitly cancel and await the remaining tasks.
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
| offset: int | ||
|
|
||
|
|
||
| async def anonymizer_chat_compatibility( |
There was a problem hiding this comment.
vLLM registers its authentication middleware before custom middleware, while Starlette makes later-added middleware outermost. Because this handler returns directly for /v1/chat/completions without calling call_next, it appears that requests handled here never reach vLLM’s API-key check.
This matters because the deployment guidance suggests configuring api_key_env; users may reasonably expect that to protect detector requests. Could we either enforce the configured bearer token in this middleware or add an integration test showing that missing/incorrect tokens receive 401 while the correct token succeeds?
There was a problem hiding this comment.
Fixed in 8562bdd.
- The compatibility middleware now validates
VLLM_API_KEYbefore request parsing or inference. - Missing, malformed, and incorrect credentials return vLLM's
401 {"error":"Unauthorized"}response. - Correct Bearer credentials and disabled authentication continue to succeed.
- Regression tests cover all five cases.
Validation: focused tests passed (58 passed, 3 skipped), make format-check passed, make typecheck passed, and the full suite passed (1,341 passed, 3 skipped).
| return _stop_running_handle(handle, timeout_seconds).cleanup_complete | ||
|
|
||
|
|
||
| def _stop_running_handle( |
There was a problem hiding this comment.
After sending SIGTERM to the process group, the shutdown loop calls is_handle_running(handle), which checks the recorded leader PID. If the leader exits while one of its vLLM worker processes remains alive, cleanup can report cleanup_complete=True and skip the SIGKILL fallback.
Could we verify that the entire process group has exited before reporting successful cleanup? This seems particularly important for GPU workers, since a surviving child could continue holding GPU memory even though stop reports success.
|
Can we preserve the native PyTorch GLiNER serving path as an optional, installable component rather than deleting it entirely? The vLLM Factory path makes sense for Linux/NVIDIA GPU deployments, but it does not work for the existing Mac/MPS and CPU use cases. I'm thinking about this from the perspective of fixing up the tutorial notebooks, where we want users to install something like Since |
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Summary
Scope and lifecycle
The tool owns one deployment domain: a managed local-process vLLM server.
task,model,vllm, andlocal.compile,launch,probe,status, andstop.VLLM_API_KEYbefore request parsing or inference and returns401for missing, malformed, or incorrect credentials.The tool remains under
tools/and is not part of the distributable Anonymizer package. It does not attach to existing endpoints or manage remote compute.Validation
Validation on current HEAD
8562bdd:make format-check: passed.make typecheck: passed.make test: 1,341 passed and 3 expected skips.Earlier full-branch validation on rebased HEAD
836a13f:make check: passed, including formatting, lint,ty, lock consistency, and SPDX checks.make docs-build: strict build passed.main.Runtime evidence from the patch-equivalent pre-rebase tree:
tools/inference_service.Dockerfileas a real GPU image. The packaged CLI exposed all lifecycle commands and the vLLM Factory compatibility doctor passed.docs/data/NVIDIA_synthetic_biographies.csvwith zero workflow failures.cleanup_complete=true; final A100 memory use was 0 MiB.Fresh NVSkills validation remains requested so generated skill artifacts and signatures bind to the current commit chain.
Fixes #252