feat: Minor improvements to build.py #8362
Conversation
kpedro88
force-pushed
the
buildpy_fixes
branch
from
1e2cbd3 to
f940474
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the build.py script with four new command-line flags to increase build flexibility, particularly for development workflows and external contributors working with forks.
- Adds
--no-container-cacheflag to disable Docker cache during container builds - Adds
--default-repo-tagflag to override calculated default repository tags globally - Adds
--use-buildbaseflag to use the temporary "buildbase" image for backend builds - Extends
--backendsyntax to support custom organizations/repositories
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| build.py | Implements the four new flags and their associated logic, including regex parsing for backend syntax and image mapping updates |
| docs/customization_guide/build.md | Documents the new --use-buildbase flag and extended backend syntax with organization support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pattern = r"(https?:\/\/[^\s:]+)|:" | ||
| parts = list(filter(None,re.split(pattern, be))) |
There was a problem hiding this comment.
The regex pattern and splitting logic is complex and may be difficult to maintain. Consider extracting this parsing logic into a separate function with clear documentation about the expected input format and returned structure.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Agreed. My followup branch is not quite finished yet, but involves a more thorough redesign that will avoid the need for any custom parsing.
| for be in FLAGS.backend: | ||
| parts = be.split(":") | ||
| pattern = r"(https?:\/\/[^\s:]+)|:" | ||
| parts = list(filter(None,re.split(pattern, be))) |
There was a problem hiding this comment.
Missing space after comma in function call. Should be filter(None, re.split(pattern, be)).
| parts = list(filter(None,re.split(pattern, be))) | |
| parts = list(filter(None, re.split(pattern, be))) |
Copilot uses AI. Check for mistakes.
|
Thanks for the contribution! I know we've been distracted, but we're doing our best to catch back up. This looks good to me. @mc-nv is the expert here. I believe he's out of office until next week, hopefully after he's back and the dust settled, I can get his review here. |
whoisj
added
enhancement
What does the PR do?
This PR adds three new flags and updates one flag in
build.py. These changes all go toward increasing build flexibility.--no-container-cache, which propagates todocker --no-cache.--default-repo-tag <tag>to override the calculated default value, which is not always appropriate. For example, when trying to build a dev version (currently 25.08), the upstream container version is set to the previous version (25.07), which takes precedence in the calculated default value, but using the corresponding dev versions of component and backend repositories may be intended. Rather than having to override it for each repo, it is useful to be able to override it globally.--use-buildbaseto use the temporary "buildbase" image as the "base" image for backends that need it (e.g. onnxruntime).--backendsyntax to<backend-name>[:<repo-tag>][:<org>]to allow specifying a different organization/repository, in addition to a different tag/branch. This is useful for external contributors developing in forks.Checklist
Agreement
<commit_type>: <Title>pre-commit install, pre-commit run --all)Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
Related PRs:
N/A
Where should the reviewer start?
Changes only affect build.py and associated documentation.
Test plan:
These changes only affect the build script, so there is no impact on server functionality.
An example of using some of these options:
in branch
r25.08(without these changes):output:
Building Triton Inference Server
platform linux
machine x86_64
version 2.60.0dev
build dir /storage/local/data2/pedrok/sonic/server/build
install dir None
cmake dir None
default repo-tag: r25.07
container version 25.08dev
upstream container version 25.07
endpoint "http"
endpoint "grpc"
endpoint "sagemaker"
endpoint "vertex-ai"
filesystem "gcs"
filesystem "s3"
filesystem "azure_storage"
backend "ensemble" at tag/branch "r25.07"
backend "identity" at tag/branch "r25.07"
backend "square" at tag/branch "r25.07"
backend "repeat" at tag/branch "r25.07"
backend "onnxruntime" at tag/branch "r25.07"
backend "python" at tag/branch "r25.07"
backend "dali" at tag/branch "r25.07"
backend "pytorch" at tag/branch "r25.07"
backend "openvino" at tag/branch "r25.07"
backend "fil" at tag/branch "r25.07"
backend "tensorrt" at tag/branch "r25.07"
repoagent "checksum" at tag/branch "r25.07"
cache "local" at tag/branch "r25.07"
cache "redis" at tag/branch "r25.07"
component "common" at tag/branch "r25.07"
component "core" at tag/branch "r25.07"
component "backend" at tag/branch "r25.07"
component "thirdparty" at tag/branch "r25.07"
After merging this branch into
r25.08, the following is possible:output:
Building Triton Inference Server
platform linux
machine x86_64
version 2.60.0dev
build dir /storage/local/data2/pedrok/sonic/server/build
install dir None
cmake dir None
default repo-tag: r25.08
container version 25.08dev
upstream container version 25.07
endpoint "http"
endpoint "grpc"
endpoint "sagemaker"
endpoint "vertex-ai"
filesystem "gcs"
filesystem "s3"
filesystem "azure_storage"
backend "onnxruntime" at tag/branch "r25.08_kjp" from org "https://github.com/kpedro88"
backend "ensemble" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "identity" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "square" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "repeat" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "python" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "dali" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "pytorch" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "openvino" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "fil" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
backend "tensorrt" at tag/branch "r25.08" from org "https://github.com/triton-inference-server"
repoagent "checksum" at tag/branch "r25.08"
cache "local" at tag/branch "r25.08"
cache "redis" at tag/branch "r25.08"
component "common" at tag/branch "r25.08"
component "core" at tag/branch "r25.08"
component "backend" at tag/branch "r25.08"
component "thirdparty" at tag/branch "r25.08"
Caveats:
The syntax for changing the backend org could be more elegant, and the feature is not extended to other components besides backends. I am planning a more thorough improvement to
build.pythat will address this in a followup PR, but it will be a more involved change and is still in progress.Background
These changes were useful in building a
25.08devserver to incorporate #8335, which includes an important bug fix and is not yet in a released version.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
N/A