Skip to content

feat(cli): add model artifact inventory pipeline - #457

Open
goingforstudying-ctrl wants to merge 5 commits into
NVIDIA:mainfrom
goingforstudying-ctrl:goingforstudying-ctrl/docs/function-task-pipeline
Open

feat(cli): add model artifact inventory pipeline#457
goingforstudying-ctrl wants to merge 5 commits into
NVIDIA:mainfrom
goingforstudying-ctrl:goingforstudying-ctrl/docs/function-task-pipeline

Conversation

@goingforstudying-ctrl

@goingforstudying-ctrl goingforstudying-ctrl commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

TL;DR

Add a runnable self-hosted reference architecture that combines a long-running
NVCF Function with a run-to-completion NVIDIA Cloud Task to inventory model and
dataset artifacts.

Additional Details

This change adds:

  • A client-orchestrated admission, Task, and completion sequence where the
    Function-returned artifact references drive the Task mounts.
  • A standard-library Python Task image that produces a deterministic SHA-256
    inventory and uploads report.json.
  • Production-aligned result selection for worker-task's
    artifact-inventory_<uuid> NGC version naming.
  • Shared-volume-safe model and dataset subdirectory selection, including
    traversal and collision guards for admitted artifact names.
  • Bounded Task status, event, and result requests through task get --timeout,
    task events --timeout, and task results --timeout, with independent event
    and result deadlines and bounded transient status-read retries.
  • Mode-600, non-argv/non-child-environment handling for the NGC write key.
  • Hermetic CLI package-test isolation so credential and saved-state mutations
    cannot leak into the admin assertions exercised by the expanded test suite.
  • Deadline-aware polling, terminal-state handling, and bounded cleanup retries.
  • Regression coverage for HTTP payloads/timeouts, accepted artifact handoff,
    shared-volume isolation, upload metadata, result predicates, deterministic
    hashing, symlinks, malformed IDs, transient reads, and cleanup behavior.

The Task image reuses python:3.12-bullseye and only the Python standard
library. It adds no package dependency.

For the Reviewer

Please focus on:

  • Whether the concrete Function-to-Task artifact handoff addresses Publish reference architectures that combine Functions and Tasks #115.
  • Whether the shared-volume artifact isolation and uploaded inventory metadata
    match worker-task behavior.
  • Whether the timeout, retry, and cleanup behavior are suitable automation
    primitives.
  • Whether the secret handling and artifact-file checks are appropriate.

For QA

Local validation on the final tree:

  • python3 -m unittest discover -s examples/reference-architectures/function-task-pipeline/task -p 'test_*.py' -v (10/10)
  • bash examples/reference-architectures/function-task-pipeline/test_run.sh
  • go test ./cmd -count=1 from src/clis/nvcf-cli
  • go test ./cmd -run 'TestTask(Events|Get|Results)TimeoutCancelsHTTPRequest' -count=10 from src/clis/nvcf-cli
  • go test -race ./... -count=1 from src/clis/nvcf-cli
  • go vet ./... from src/clis/nvcf-cli
  • bazel test --remote_cache= --nocache_test_results //src/clis/nvcf-cli/... --test_output=errors (21/21)
  • bazel build --remote_cache= //src/clis/nvcf-cli/...
  • bash -n examples/reference-architectures/function-task-pipeline/run.sh examples/reference-architectures/function-task-pipeline/test_run.sh
  • ./tools/ci/check-docs (Fern: 0 errors, 1 warning; advisory for absent OSS-mirror imports.yaml)
  • ./tools/ci/check-license
  • git diff --check

Three independent final-diff reviews covered implementation correctness and
edge cases; regression quality and failure reproduction; and repository scope,
security, API claims, licensing, and overlap. All concluded with no actionable
findings after fixes.

Issues

Closes #115

Dependencies

None. The new Task image uses the Python standard library and the same Python
base-image family already used by repository examples. No NOTICE update is
needed.

Checklist

  • I am familiar with the Contributing Guidelines.
  • I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The CLI gains JSON-safe function creation output, resource ID validation, and optional task retrieval timeouts. A new reference architecture combines a Function with a run-to-completion Task for artifact inventory, result validation, completion handling, cleanup, and deterministic testing.

Changes

CLI behavior and documentation

Layer / File(s) Summary
Function creation output and state validation
src/clis/nvcf-cli/cmd/function.go, src/clis/nvcf-cli/cmd/function_create_test.go, src/clis/nvcf-cli/cmd/BUILD.bazel
Function creation validates IDs, supports structured JSON output, and adds end-to-end coverage for valid and invalid function/task responses.
Task timeout and identifier validation
src/clis/nvcf-cli/cmd/task.go, src/clis/nvcf-cli/cmd/task_test.go
Task inspection commands accept bounded timeouts. Task creation rejects responses without a task ID. Tests cover deadlines, cancellation, flags, and payload handling.
CLI usage documentation
docs/user/cli.md, src/clis/nvcf-cli/README.md, src/clis/nvcf-cli/USAGE-GUIDE.md
Examples document JSON function creation and bounded task retrieval.
CLI test state isolation
src/clis/nvcf-cli/cmd/admin_test.go
Admin tests use temporary configuration and restore package-level state during cleanup.

Function and Task pipeline reference architecture

Layer / File(s) Summary
Reference architecture overview and setup
examples/README.md, examples/reference-architectures/function-task-pipeline/README.md
Documents the combined workflow, prerequisites, image publishing, configuration, execution, failure handling, and validation commands.
Artifact inventory Task
examples/reference-architectures/function-task-pipeline/task/*
Builds a non-root Python Task image that inventories mounted artifacts, computes SHA-256 metadata, writes report and progress JSON, and validates filesystem inputs.
Pipeline orchestration and cleanup
examples/reference-architectures/function-task-pipeline/run.sh
Creates and deploys a Function, performs admission, creates and polls a Task, validates results, invokes completion, emits aggregate JSON, and cleans up resources.
Deterministic workflow validation
examples/reference-architectures/function-task-pipeline/test_run.sh, examples/reference-architectures/function-task-pipeline/task/test_inventory_artifacts.py
Mocks CLI operations and tests sequencing, admission and result checks, terminal failures, timeouts, identifier validation, cleanup retries, secret handling, and filesystem validation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WorkflowClient
  participant NVCFFunction
  participant NVCTAPI
  participant TaskWorker
  WorkflowClient->>NVCFFunction: invoke admission stage
  WorkflowClient->>NVCTAPI: create batch task with accepted artifacts
  NVCTAPI->>TaskWorker: execute artifact inventory
  WorkflowClient->>NVCTAPI: poll status and retrieve validated results
  WorkflowClient->>NVCFFunction: invoke completion stage
  WorkflowClient->>WorkflowClient: emit aggregated JSON and clean up
Loading

Suggested reviewers: pdmack, sbaum1994

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Most changes support the reference architecture, but unrelated admin test-state isolation changes are included in the pull request. Move the unrelated admin test scaffolding changes to a separate pull request, unless they are required to support this reference architecture.
Docstring Coverage ⚠️ Warning Docstring coverage is 17.39% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The pull request adds a runnable workflow that combines an NVCF Function and run-to-completion Task as required by issue #115.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits and accurately describes the primary feature: a model artifact inventory pipeline.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@goingforstudying-ctrl goingforstudying-ctrl changed the title docs(examples): add function and task pipeline feat(cli): add robust function and task pipeline Jul 26, 2026
@goingforstudying-ctrl
goingforstudying-ctrl marked this pull request as ready for review July 26, 2026 04:36
@goingforstudying-ctrl
goingforstudying-ctrl requested review from a team as code owners July 26, 2026 04:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
examples/reference-architectures/function-task-pipeline/test_run.sh (1)

86-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a test for the task cancel failure path.

The fake CLI already supports FAKE_TASK_CANCEL_FAIL to simulate a failed cancel, but no scenario in this file exercises it. This leaves cleanup()'s cancel-failure branch (run.sh Lines 80-83, which sets cleanup_failed=true) unverified.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/reference-architectures/function-task-pipeline/test_run.sh` around
lines 86 - 90, Add a test scenario in test_run.sh that sets
FAKE_TASK_CANCEL_FAIL=true, invokes the cleanup flow through task cancel, and
asserts the command fails while cleanup_failed is set as expected. Reuse the
existing test setup and teardown patterns, and leave the fake CLI handling in
the task cancel case unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/clis/nvcf-cli/cmd/task.go`:
- Around line 441-453: Update newTaskGetContext to reject timeoutSeconds values
above 9,223,372,036 before converting them to time.Duration, returning the
existing validation error for invalid values while preserving the zero-timeout
and valid positive-timeout behavior.

---

Nitpick comments:
In `@examples/reference-architectures/function-task-pipeline/test_run.sh`:
- Around line 86-90: Add a test scenario in test_run.sh that sets
FAKE_TASK_CANCEL_FAIL=true, invokes the cleanup flow through task cancel, and
asserts the command fails while cleanup_failed is set as expected. Reuse the
existing test setup and teardown patterns, and leave the fake CLI handling in
the task cancel case unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8f12b63a-a764-460a-b70f-c04c4973f30b

📥 Commits

Reviewing files that changed from the base of the PR and between 1c29b4b and 1eaf634.

📒 Files selected for processing (12)
  • docs/user/cli.md
  • examples/README.md
  • examples/reference-architectures/function-task-pipeline/README.md
  • examples/reference-architectures/function-task-pipeline/run.sh
  • examples/reference-architectures/function-task-pipeline/test_run.sh
  • src/clis/nvcf-cli/README.md
  • src/clis/nvcf-cli/USAGE-GUIDE.md
  • src/clis/nvcf-cli/cmd/BUILD.bazel
  • src/clis/nvcf-cli/cmd/function.go
  • src/clis/nvcf-cli/cmd/function_create_test.go
  • src/clis/nvcf-cli/cmd/task.go
  • src/clis/nvcf-cli/cmd/task_test.go

Comment thread src/clis/nvcf-cli/cmd/task.go Outdated

@FamousDirector FamousDirector left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fine contribution at a high level, but it lacks any real sample. It shows how to combine tasks and functions, but not what workflow might utilize it, or how data might be transferred between them. A finetuning/RL model pipeline for example would be a good sample workflow that demonstrates the reference arch beyond something trivial.

Signed-off-by: goingforstudying-ctrl <goingforstudying@gmail.com>
Signed-off-by: goingforstudying-ctrl <goingforstudying@gmail.com>
Cover task cancellation cleanup failures in the reference workflow tests.

Relates to NVIDIA#115

Signed-off-by: goingforstudying-ctrl <goingforstudying@gmail.com>
Address maintainer feedback with a concrete Function-to-Task artifact workflow, bounded result polling, and production-aligned upload metadata.

Relates to NVIDIA#115

Signed-off-by: goingforstudying-ctrl <goingforstudying@gmail.com>
@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the goingforstudying-ctrl/docs/function-task-pipeline branch from 9636b9c to de531af Compare July 30, 2026 06:14
@goingforstudying-ctrl goingforstudying-ctrl changed the title feat(cli): add robust function and task pipeline feat(cli): add model artifact inventory pipeline Jul 30, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (6)
examples/reference-architectures/function-task-pipeline/task/inventory_artifacts.py (1)

25-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Wrap required env-var lookups with clearer errors.

os.environ["WORKFLOW_REQUEST_BASE64"] (line 26) and os.environ["RESULTS_LOCATION"] (line 44) will raise a bare KeyError if unset, which surfaces as an unhelpful traceback in Task logs. Since this is a reference architecture meant to illustrate failure boundaries, a clearer error would aid operators diagnosing misconfiguration.

♻️ Proposed fix
+def _require_env(name: str) -> str:
+    try:
+        return os.environ[name]
+    except KeyError as error:
+        raise ValueError(f"required environment variable {name} is not set") from error
+
+
 def load_config() -> Config:
-    encoded_request = os.environ["WORKFLOW_REQUEST_BASE64"]
+    encoded_request = _require_env("WORKFLOW_REQUEST_BASE64")
     try:
         decoded_request = base64.b64decode(encoded_request, validate=True)
         workflow_request = json.loads(decoded_request)
     except (ValueError, json.JSONDecodeError) as error:
         raise ValueError("WORKFLOW_REQUEST_BASE64 must encode a JSON object") from error
     if not isinstance(workflow_request, dict):
         raise ValueError("WORKFLOW_REQUEST_BASE64 must encode a JSON object")

     results_dir = Path(os.getenv("NVCT_RESULTS_DIR", "/var/task/result"))
     return Config(
         models_dir=Path(os.getenv("INPUT_MODELS_DIR", "/config/models")),
         resources_dir=Path(os.getenv("INPUT_RESOURCES_DIR", "/config/resources")),
         results_dir=results_dir,
         progress_file=Path(
             os.getenv("NVCT_PROGRESS_FILE_PATH", str(results_dir / "progress"))
         ),
         task_id=os.getenv("NVCT_TASK_ID", ""),
-        results_location=os.environ["RESULTS_LOCATION"],
+        results_location=_require_env("RESULTS_LOCATION"),
         workflow_request=workflow_request,
     )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@examples/reference-architectures/function-task-pipeline/task/inventory_artifacts.py`
around lines 25 - 46, Update load_config to handle missing
WORKFLOW_REQUEST_BASE64 and RESULTS_LOCATION environment variables with clear,
descriptive configuration errors instead of bare KeyError exceptions. Preserve
the existing defaults for optional variables and continue constructing Config
with the validated required values.
examples/reference-architectures/function-task-pipeline/test_run.sh (3)

149-150: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Quote the RHS in [[ ... != ... ]] to force literal comparison.

Inside [[ ]] an unquoted right-hand side is a glob pattern. The current values contain no metacharacters so behavior is correct today, but a future artifact value with *, ?, or [ would silently turn this assertion into a pattern match.

♻️ Proposed change
-        if [[ $model_artifact != ${FAKE_EXPECTED_MODEL_ARTIFACT:-model:test:ngc://models/test} ||
-              $dataset_artifact != dataset:test:ngc://datasets/test ]]; then
+        if [[ $model_artifact != "${FAKE_EXPECTED_MODEL_ARTIFACT:-model:test:ngc://models/test}" ||
+              $dataset_artifact != "dataset:test:ngc://datasets/test" ]]; then
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/reference-architectures/function-task-pipeline/test_run.sh` around
lines 149 - 150, Quote both right-hand-side artifact values in the conditional
comparison within the test script’s assertion so [[ ... != ... ]] performs
literal string comparisons rather than glob matching. Preserve the existing
default model artifact value and dataset artifact value unchanged.

746-756: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a KEEP_RESOURCES=true case.

The README documents KEEP_RESOURCES=true as a supported mode (README Lines 147-152 and 191) and run.sh branches on it in cleanup, but no test exercises it. A short case asserting the workflow succeeds and no task delete / function deploy remove / function delete appears in the log would close the gap cheaply.

♻️ Proposed addition
+reset_fake
+KEEP_RESOURCES=true run_workflow >/dev/null
+if grep -q -E -- "task delete|function deploy remove|function delete" "$fake_log"; then
+    printf 'KEEP_RESOURCES=true must not delete workflow resources\n' >&2
+    exit 1
+fi
+
 printf 'function-task-pipeline tests passed\n'

run_workflow also needs to forward the variable:

+    KEEP_RESOURCES=${KEEP_RESOURCES:-false} \
     CLEANUP_DELETE_ATTEMPTS=3 \
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/reference-architectures/function-task-pipeline/test_run.sh` around
lines 746 - 756, Add a test case in test_run.sh that sets KEEP_RESOURCES=true,
forwards it through run_workflow, asserts the workflow succeeds, and verifies
fake_log contains no task delete, function deploy remove, or function delete
commands. Keep the existing validation test unchanged.

567-572: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

unset only clears the first element of mismatch_env.

Every current case sets exactly one variable, so isolation holds today. If a case ever adds a second entry, the extra export leaks into all later iterations and silently weakens the matrix. Unsetting the whole array removes that trap.

♻️ Proposed change
     export "${mismatch_env[@]}"
     if RESULTS_TIMEOUT_SECONDS=1 run_workflow >/dev/null 2>&1; then
         printf 'Expected mismatched %s result metadata to fail\n' "$mismatch" >&2
         exit 1
     fi
-    unset "${mismatch_env[0]%%=*}"
+    for assignment in "${mismatch_env[@]}"; do
+        unset "${assignment%%=*}"
+    done
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/reference-architectures/function-task-pipeline/test_run.sh` around
lines 567 - 572, Update the cleanup after the mismatch validation in the test
loop to unset every exported entry in mismatch_env, rather than only
mismatch_env[0]. Preserve the existing per-iteration isolation by removing all
variable names represented in the array before the next case runs.
examples/reference-architectures/function-task-pipeline/run.sh (2)

192-198: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Report the orphaned function when ID extraction fails.

If function create succeeds but the response lacks a usable id/versionId, the script exits via errexit with function_created=false, so cleanup skips deletion and the function is leaked with no way for the operator to locate it. Emitting the create response (or at least the function name) on extraction failure keeps this consistent with the cleanup path that prints resource IDs for manual inspection.

♻️ Proposed change
-function_id=$(jq -er \
-    '.function.id | select(type == "string" and length > 0)' \
-    <<<"$function_create_json")
-version_id=$(jq -er \
-    '.function.versionId | select(type == "string" and length > 0)' \
-    <<<"$function_create_json")
+if ! function_id=$(jq -er \
+    '.function.id | select(type == "string" and length > 0)' \
+    <<<"$function_create_json") ||
+    ! version_id=$(jq -er \
+        '.function.versionId | select(type == "string" and length > 0)' \
+        <<<"$function_create_json"); then
+    log "ERROR: function create returned no usable identifiers; verify function ${function_name} manually"
+    fail "Unusable function create response"
+fi
 function_created=true
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/reference-architectures/function-task-pipeline/run.sh` around lines
192 - 198, Update the function ID and version ID extraction in the function
creation flow to report the create response or function name before errexit
terminates when either value is unusable. Preserve the existing successful
extraction and cleanup behavior, while ensuring operators receive enough
identifying information to locate an orphaned function.

288-295: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

A single transient task get failure aborts the whole workflow.

Any non-deadline CLI error (network blip, 5xx) fails the run and triggers Task cancellation even though the Task is still healthy and there is remaining budget. Consider tolerating a bounded number of consecutive status-read errors before failing, consistent with the bounded-retry policy already applied in retry_cleanup.

♻️ Sketch
+status_failures=0
 while true; do
@@
     if ! task_json=$(cli --json task get \
         --timeout "$remaining_seconds" \
         "$task_id"); then
         if ((SECONDS >= task_deadline)); then
             fail "Task ${task_id} did not finish within ${TASK_TIMEOUT_SECONDS} seconds"
         fi
-        fail "Failed to read status for task ${task_id}"
+        status_failures=$((status_failures + 1))
+        if ((status_failures > STATUS_READ_ATTEMPTS)); then
+            fail "Failed to read status for task ${task_id}"
+        fi
+        log "Transient status read failure for task ${task_id} (${status_failures}/${STATUS_READ_ATTEMPTS})"
+        sleep "$POLL_INTERVAL_SECONDS"
+        continue
     fi
+    status_failures=0
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/reference-architectures/function-task-pipeline/run.sh` around lines
288 - 295, Update the task-status polling flow around cli task get to tolerate a
bounded number of consecutive transient read failures while time remains,
resetting the counter after a successful read. Reuse the existing retry limit or
policy from retry_cleanup, and only call fail once that bound is exhausted or
task_deadline is reached; preserve the existing timeout and task-status
behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@examples/reference-architectures/function-task-pipeline/task/Dockerfile`:
- Around line 4-10: Add a non-root USER directive to the task image defined by
the Dockerfile, creating or selecting an appropriate unprivileged runtime user
before the existing CMD executes. Ensure the user can access /app and externally
mounted files needed by inventory_artifacts.py, while preserving the current
application entrypoint.

In `@src/clis/nvcf-cli/cmd/admin_test.go`:
- Around line 70-81: Update the test helper setup around viper.Set("token", ...)
to also clear the inherited NVCF_TOKEN environment variable with
t.Setenv("NVCF_TOKEN", "") before configuring the Viper token, while preserving
the existing NVCF_API_KEY reset.

---

Nitpick comments:
In `@examples/reference-architectures/function-task-pipeline/run.sh`:
- Around line 192-198: Update the function ID and version ID extraction in the
function creation flow to report the create response or function name before
errexit terminates when either value is unusable. Preserve the existing
successful extraction and cleanup behavior, while ensuring operators receive
enough identifying information to locate an orphaned function.
- Around line 288-295: Update the task-status polling flow around cli task get
to tolerate a bounded number of consecutive transient read failures while time
remains, resetting the counter after a successful read. Reuse the existing retry
limit or policy from retry_cleanup, and only call fail once that bound is
exhausted or task_deadline is reached; preserve the existing timeout and
task-status behavior.

In
`@examples/reference-architectures/function-task-pipeline/task/inventory_artifacts.py`:
- Around line 25-46: Update load_config to handle missing
WORKFLOW_REQUEST_BASE64 and RESULTS_LOCATION environment variables with clear,
descriptive configuration errors instead of bare KeyError exceptions. Preserve
the existing defaults for optional variables and continue constructing Config
with the validated required values.

In `@examples/reference-architectures/function-task-pipeline/test_run.sh`:
- Around line 149-150: Quote both right-hand-side artifact values in the
conditional comparison within the test script’s assertion so [[ ... != ... ]]
performs literal string comparisons rather than glob matching. Preserve the
existing default model artifact value and dataset artifact value unchanged.
- Around line 746-756: Add a test case in test_run.sh that sets
KEEP_RESOURCES=true, forwards it through run_workflow, asserts the workflow
succeeds, and verifies fake_log contains no task delete, function deploy remove,
or function delete commands. Keep the existing validation test unchanged.
- Around line 567-572: Update the cleanup after the mismatch validation in the
test loop to unset every exported entry in mismatch_env, rather than only
mismatch_env[0]. Preserve the existing per-iteration isolation by removing all
variable names represented in the array before the next case runs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0bd7dd6f-f446-407d-b4fa-85add432e0cb

📥 Commits

Reviewing files that changed from the base of the PR and between 9636b9c and de531af.

📒 Files selected for processing (16)
  • docs/user/cli.md
  • examples/README.md
  • examples/reference-architectures/function-task-pipeline/README.md
  • examples/reference-architectures/function-task-pipeline/run.sh
  • examples/reference-architectures/function-task-pipeline/task/Dockerfile
  • examples/reference-architectures/function-task-pipeline/task/inventory_artifacts.py
  • examples/reference-architectures/function-task-pipeline/task/test_inventory_artifacts.py
  • examples/reference-architectures/function-task-pipeline/test_run.sh
  • src/clis/nvcf-cli/README.md
  • src/clis/nvcf-cli/USAGE-GUIDE.md
  • src/clis/nvcf-cli/cmd/BUILD.bazel
  • src/clis/nvcf-cli/cmd/admin_test.go
  • src/clis/nvcf-cli/cmd/function.go
  • src/clis/nvcf-cli/cmd/function_create_test.go
  • src/clis/nvcf-cli/cmd/task.go
  • src/clis/nvcf-cli/cmd/task_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • examples/README.md
  • docs/user/cli.md
  • src/clis/nvcf-cli/README.md

Comment thread src/clis/nvcf-cli/cmd/admin_test.go
@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the goingforstudying-ctrl/docs/function-task-pipeline branch from 3ad2531 to 03b2eec Compare August 1, 2026 05:32

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
src/clis/nvcf-cli/cmd/task_test.go (1)

172-175: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Exercise the --timeout flag in the cancellation test.

Line 172 mutates taskPaginationFlags.timeoutSeconds directly. This bypasses Cobra flag binding. A registered flag could stop updating the backing field while this test still passes. Set the flag through taskEventsCmd.Flags().Set("timeout", "1") or execute the command with --timeout 1, then restore the flag value.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/clis/nvcf-cli/cmd/task_test.go` around lines 172 - 175, Update the
cancellation test around runTaskEvents to set the timeout through Cobra’s
taskEventsCmd.Flags().Set("timeout", "1") (or pass --timeout 1) instead of
mutating taskPaginationFlags.timeoutSeconds directly, and restore the flag value
afterward.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@examples/reference-architectures/function-task-pipeline/task/inventory_artifacts.py`:
- Around line 25-29: Update require_env to reject values that are empty or
contain only whitespace, while preserving the existing missing-variable error
behavior; raise a clear ValueError for invalid required values. Add a regression
test covering an empty RESULTS_LOCATION and verify it is rejected before task
completion.

In `@examples/reference-architectures/function-task-pipeline/test_run.sh`:
- Around line 814-833: Extend the empty Function IDs test after the workflow
failure assertion to verify that `fake_log` does not contain `function deploy
create`, matching the malformed create-response check. Ensure the test fails if
deployment is attempted with empty IDs.
- Around line 157-158: Update run_workflow() and the fake task-create validation
to parameterize the expected dataset artifact via
FAKE_EXPECTED_DATASET_ARTIFACT, defaulting to dataset:test:ngc://datasets/test
when unset, matching the existing FAKE_EXPECTED_MODEL_ARTIFACT behavior. Replace
the hard-coded dataset comparison while preserving the current model validation.

In `@src/clis/nvcf-cli/cmd/task_test.go`:
- Around line 174-178: Update the timeout test around runTaskEvents to avoid
asserting that the entire call completes within two seconds, since setup occurs
before the request deadline. Use the handler’s cancellation signal with a
generous overall test timeout, or measure only the request phase, while
preserving verification of context.DeadlineExceeded.

---

Nitpick comments:
In `@src/clis/nvcf-cli/cmd/task_test.go`:
- Around line 172-175: Update the cancellation test around runTaskEvents to set
the timeout through Cobra’s taskEventsCmd.Flags().Set("timeout", "1") (or pass
--timeout 1) instead of mutating taskPaginationFlags.timeoutSeconds directly,
and restore the flag value afterward.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 86088aad-b306-46e6-877a-3e638559a89a

📥 Commits

Reviewing files that changed from the base of the PR and between 3ad2531 and 03b2eec.

📒 Files selected for processing (13)
  • docs/user/cli.md
  • examples/README.md
  • examples/reference-architectures/function-task-pipeline/README.md
  • examples/reference-architectures/function-task-pipeline/run.sh
  • examples/reference-architectures/function-task-pipeline/task/Dockerfile
  • examples/reference-architectures/function-task-pipeline/task/inventory_artifacts.py
  • examples/reference-architectures/function-task-pipeline/task/test_inventory_artifacts.py
  • examples/reference-architectures/function-task-pipeline/test_run.sh
  • src/clis/nvcf-cli/README.md
  • src/clis/nvcf-cli/USAGE-GUIDE.md
  • src/clis/nvcf-cli/cmd/admin_test.go
  • src/clis/nvcf-cli/cmd/task.go
  • src/clis/nvcf-cli/cmd/task_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
  • docs/user/cli.md
  • src/clis/nvcf-cli/USAGE-GUIDE.md
  • examples/reference-architectures/function-task-pipeline/task/Dockerfile
  • examples/README.md
  • src/clis/nvcf-cli/README.md
  • src/clis/nvcf-cli/cmd/admin_test.go
  • examples/reference-architectures/function-task-pipeline/README.md
  • src/clis/nvcf-cli/cmd/task.go
  • examples/reference-architectures/function-task-pipeline/run.sh

Comment thread examples/reference-architectures/function-task-pipeline/test_run.sh Outdated
Comment thread examples/reference-architectures/function-task-pipeline/test_run.sh
Comment thread src/clis/nvcf-cli/cmd/task_test.go Outdated
Address review findings by isolating model and dataset directories on the shared artifact volume, bounding event and status reads, and strengthening failure diagnostics and regression coverage.

Relates to NVIDIA#115

Signed-off-by: goingforstudying-ctrl <goingforstudying@gmail.com>
@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the goingforstudying-ctrl/docs/function-task-pipeline branch from 03b2eec to c1a1f90 Compare August 1, 2026 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Publish reference architectures that combine Functions and Tasks

2 participants