AIR CLI: drop max_retries from the ai_runtime_task payload#5813
Open
riddhibhagwat-db wants to merge 1 commit into
Open
AIR CLI: drop max_retries from the ai_runtime_task payload#5813riddhibhagwat-db wants to merge 1 commit into
riddhibhagwat-db wants to merge 1 commit into
Conversation
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Contributor
Waiting for approvalCould not determine reviewers from git history. Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
Collaborator
Integration test reportCommit: b25accb
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 28 slowest tests (at least 2 minutes):
|
riddhibhagwat-db
force-pushed
the
air-max-retries-fix
branch
from
3de55a3 to
f5eb71b
Compare
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
maggiewang-db
left a comment
maggiewang-db
left a comment
There was a problem hiding this comment.
execution retries are driven by the AI Runtime service (AICM) - Is it true?
@ben-hansen-db How is a user supposed to specify the max retry number with the new BYOT?
The ai_runtime_task path DOES honor the Jobs task max_retries field — Jobs performs the retry and each attempt is created as a fresh AI Runtime (AICM) workload (see ai-training AiTrainingHandlers: "Each Jobs retry attempt is created as its own AICM workload"). So the retry knob is real; the earlier assumption that it had no effect was wrong. The actual mismatch with the Python CLI was that we always sent max_retries (including an explicit 0) and derived retry_on_timeout from it. Python's native (`is_ts`) path adds both fields only under `if max_retries > 0`. Match that exactly: - max_retries/retry_on_timeout are omitempty and set only when cfg.maxRetries() > 0. - Unset max_retries still defaults to 3 (Python's default via sdk/config.py Field(default=3)), so plain `air run` sends 3. - max_retries: 0 now omits both fields (server default / no retries), instead of sending "max_retries": 0. Co-authored-by: Isaac
riddhibhagwat-db
force-pushed
the
air-max-retries-fix
branch
from
f5eb71b to
b25accb
Compare
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
riddhibhagwat-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request and its description were written by Isaac.
Changes
Stop sending
max_retries/retry_on_timeouton theai_runtime_tasksubmit payload. Both fields are removed from thesubmitTaskstruct and frombuildSubmitPayload. Themax_retriesYAML config field and its validation stay in the schema; it is simply no longer put on the wire for this submission path.Why
On the
ai_runtime_taskpath, execution retries are driven by the AI Runtime service (AICM), not by the Jobs taskmax_retriesfield — so setting it on the task had no effect on how many times a failing workload actually retried.Observed directly: a run submitted with
max_retries: 0still made 4 attempts (3 retries).air geton that run confirmed the field round-tripped correctly (Max Retries 0) while the workload retried anyway. The Python CLI's nativeai_runtime_taskbranch omitsmax_retriesfor the same reason; this matches that behavior.Leaving the field on the payload was misleading: a user setting
max_retries: 0to disable retries would still see retries.Tests
TestBuildSubmitPayloadasserts the marshaled task contains neithermax_retriesnorretry_on_timeout.TestBuildSubmitPayload_NoRetries(it asserted the now-removed field was sent).go test ./experimental/air/...and./task lint-qpass.