Repository navigation
feat(triton): [cherry-pick 1.5.0] deliver dynamo.triton and its CI (#13774) (#14760) - #14827
Conversation
…13774) (#14760) Cherry-pick of two main commits, squashed: 0027a8e feat(triton): Deliver dynamo.triton Functionality (#13774) f2becb5 fix(ci): pass framework to Triton test workflows (#14760) #13774 carries the work merged as #13943, which landed on the jwyman/dynamo-triton feature branch rather than main. Two files needed manual resolution against the release branch, both taken from main: .github/actions/pytest-local/action.yml and .github/workflows/shared-test.yml. Both hold restructuring the release branch had not seen, so the resolution changes pytest-local for every test suite on 1.5.0, not only Triton. Signed-off-by: Dan Gil <dagil@nvidia.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
github-actions
Bot
added
feat
documentation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c92e21c4ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| python - <<'PY' | ||
| import sys |
There was a problem hiding this comment.
Every CPU and GPU test stage in .github/workflows/shared-test.yml invokes this composite action, but this run block opens both a python - <<'PY' heredoc and an if without adding the terminating PY and fi. Bash parses the entire block even when the GPU condition is false, so bash -n reports an unexpected end of file and all affected CI test jobs fail before pytest starts.
Useful? React with 👍 / 👎.
| add_argument( | ||
| repo_group, | ||
| flag_name="--backend-config", | ||
| env_var="DYN_TRITON_BACKEND_CONFIG", | ||
| default=None, | ||
| dest="backend_configuration", | ||
| action=ThreeArgsDictAction, |
There was a problem hiding this comment.
When DYN_TRITON_BACKEND_CONFIG is set, add_argument installs its raw string as the argparse default, but ThreeArgsDictAction only runs for command-line occurrences. Consequently to_server_options() passes a string where Triton expects a nested dictionary; combining the environment variable with a CLI occurrence also crashes when the action calls .get() on that string. The same defect affects the environment forms of metrics, cache, host-policy, and CUDA-memory-pool configuration.
Useful? React with 👍 / 👎.
|
|
||
| self._validate_request(request) | ||
|
|
||
| inference_request = self._model.create_request() |
There was a problem hiding this comment.
When a KServe client supplies a request ID, the frontend includes it in the Dynamo tensor request, but the handler creates the Triton request without forwarding that ID and later builds the response from inference_response.request_id. The original ID therefore never reaches Triton, so clients receive an empty or independently generated response ID rather than the value they use to correlate concurrent requests.
Useful? React with 👍 / 👎.
| async for inference_response in inference_responses: | ||
| response_tensors = [] |
There was a problem hiding this comment.
When Triton returns an inference error—for example, from an invalid tensor shape or a backend execution failure—the async API yields an error-bearing response that must be checked before consuming outputs. This loop never invokes the response error check and instead serializes whatever outputs are present, so an error response with no outputs becomes a successful KServe response containing an empty tensor list, hiding the backend failure from the caller.
Useful? React with 👍 / 👎.
| triton-build: | ||
| name: triton-runtime | ||
| needs: [config] | ||
| if: github.event_name != 'workflow_dispatch' || needs.config.outputs.run_triton == 'true' | ||
| uses: $/.github/workflows/shared-build-image.yml |
There was a problem hiding this comment.
The new post-merge Triton build uses the shared b-${{ github.run_id }}-${{ github.run_attempt }} BuildKit builder, but clean-k8s-builder does not list triton-build among its dependencies. On a Triton-only workflow dispatch, the other framework jobs are skipped and cleanup can remove that builder while this job is still running; the analogous PR cleanup list also omits the new Triton jobs. Add the Triton build/test dependency before allowing cleanup to start.
Useful? React with 👍 / 👎.
| # Triton worker (serves every model in the repo; here the bundled identity sample) | ||
| python3 -m dynamo.triton --discovery-backend=file & |
There was a problem hiding this comment.
Following this quick start without mounting a custom repository starts the worker with its /models default, but the image only creates that directory empty; the bundled identity model is copied under /workspace/components/src/dynamo/triton/models. The worker therefore exits with “No ready models found” and the subsequent identity client command cannot work. Pass the bundled repository through --model-repository here or copy it into the documented default.
Useful? React with 👍 / 👎.
| async with asyncio.TaskGroup() as aio_tasks: | ||
| for name in model_names: | ||
| aio_tasks.create_task( |
There was a problem hiding this comment.
When dynamo.triton is run on Python 3.10, reaching model registration raises AttributeError because asyncio.TaskGroup was introduced in Python 3.11, while the package still declares requires-python = ">=3.10". This makes the new backend unusable on an otherwise supported interpreter as soon as at least one ready model is found; use a Python 3.10-compatible task-gathering pattern or raise the package requirement.
Useful? React with 👍 / 👎.
2 participants
Overview
Cherry-pick of the
dynamo.tritonbackend and its CI ontorelease/1.5.0. Two main commits, squashed into one:0027a8ef0bc#13774feat(triton): Deliver dynamo.triton Functionalityf2becb5fa21#14760fix(ci): pass framework to Triton test workflows#13943 is already covered: it merged into the
jwyman/dynamo-tritonfeature branch, not main, and rode in on #13774.Manual Resolution
Two files conflicted and were resolved in favor of main:
.github/actions/pytest-local/action.yml.github/workflows/shared-test.ymlBoth carry restructuring the release branch had not picked up - the env-passed pytest invocation, the
verify_gpuanddd_flaky_retry_enabledinputs, and the per-job JUnit upload. The resolution therefore changespytest-localfor every test suite on 1.5.0, not only Triton. That is the part worth a careful look.Verification
yaml.safe_loadparsespr.yaml,post-merge-ci.yml,shared-test.yml,pytest-local/action.ymlandfilters.yaml.triton-buildandtriton-testjobs present in bothpr.yamlandpost-merge-ci.yml, each withframework: tritonfrom fix(ci): pass framework to Triton test workflows #14760.🤖 Generated with Claude Code