Classify 4xx (except 429) as terminal in RestSend retry policy - #502
Open
allenrobel wants to merge 1 commit into
Open
Classify 4xx (except 429) as terminal in RestSend retry policy#502allenrobel wants to merge 1 commit into
allenrobel wants to merge 1 commit into
Conversation
allenrobel
requested review from
akinross,
anvitha-jain,
gmicol,
lhercot,
mikewiebe,
mtarking,
sajagana,
samiib and
shrsr
as code owners
August 11, 2026 23:50
Implements the shippable half of issue #457: a 4xx response proves the request reached the application and was rejected, so replaying the identical request cannot succeed. Previously a deterministic 400 (e.g. an immediately-rejected fabric mutation) was resubmitted every 5 seconds for the full 300-second budget, delaying module failure by 5 minutes with no user-facing knob to shorten it. - ResponseHandler._is_terminal_client_error(): 400-499 minus 429 (rate limiting stays retryable; no retryable 4xx is documented for any ND 4.2.1 endpoint - the dcnm-era retry-on-400 cases do not carry over). - Mutations: the #398 retryable seam now also excludes terminal client errors; 5xx keeps the historical retry behavior (the 5xx question stays open on #457). - GET: results now carry retryable, applying the same 4xx rule per the issue's scope note (a deterministic 400 on a GET burned the same budget); 5xx GETs stay retryable for eventual-consistency polling, and the 404 not-found-is-success contract is unchanged. Full unit suite: 3983 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XZiWNrVHYXbeNXRM8kUrz
allenrobel
force-pushed
the
nd_restsend_4xx_terminal
branch
from
August 13, 2026 19:24
a0dc44a to
4e2f773
Compare
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
1 participant
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.
Related Issue(s)
Addresses the 4xx half of #457 (deliberately does NOT close it — the 5xx retry question remains open for discussion there).
Proposed Changes
Implements the independently-shippable piece proposed in #457's field-report comment: a 4xx response proves the request reached the application and was rejected, so replaying the identical request cannot succeed. Previously a deterministic 400 (e.g. an immediately-rejected fabric mutation) was resubmitted every 5 seconds for the full 300-second
RestSendbudget, delaying module failure by 5 minutes with no user-facing knob to shorten it.ResponseHandler._is_terminal_client_error(): 400–499 minus 429. Rate limiting (429) is the one transient 4xx and stays retryable. No retryable 4xx is documented for any ND 4.2.1 endpoint; the dcnm-era retry-on-400 cases do not carry over.retryableclassification introduced by Detect Multi-status per-item failures in NdV1Strategy (#295) #398 now also excludes terminal client errors. 5xx keeps the historical retry behavior — that half of Discuss: should RestSend retry mutations (POST/PUT/DELETE) that fail with non-success codes? #457 is untouched.retryable, applying the same 4xx rule per the issue's scope note (a deterministic 400 on a GET burned the same budget). 5xx GETs remain retryable for eventual-consistency polling, and the 404 not-found-is-success contract is unchanged.RestSendneeds no changes — the terminal break from Detect Multi-status per-item failures in NdV1Strategy (#295) #398 already consumesretryableverb-agnostically.Test changes: five new ResponseHandler tests (01500–01540: POST 400 terminal, POST 429 retryable, GET 400 terminal, GET 404 contract, DELETE 404 terminal) and a deliberate rewrite of 01440, which previously pinned the "GET results carry no retryable key" behavior this PR removes.
Test Notes
ndpytest tests/unit/)pylint10.00/10 on the changed module;black/isortclean;mypyreports only the two errors pre-existing on developCisco Nexus Dashboard Version
4.2.1
Related ND API Resource Category
Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_019XZiWNrVHYXbeNXRM8kUrz