Aias00 commented on PR #6529:
URL: https://github.com/apache/shenyu/pull/6529#issuecomment-5193385975

   Solid, well-tested fix — the string→enum switch in 
`AbstractHttpClientPlugin` is cleaner, `acquireByName` gracefully falls back to 
`DEFAULT_BACKOFF` for null/empty/unknown (covered by 
`HttpRetryBackoffSpecEnumTest`), and 
`DivideRuleHandle.equals/hashCode/toString` are correctly updated (plus a nice 
`toString` comma fix). Removing `CustomRetryStrategy` is safe — it was a stub 
whose `execute(...)` returned `null`, and the only caller 
(`AbstractHttpClientPlugin:94`) is rewritten in this PR; no other references 
remain. Backward compat looks fine: existing rules without `retryBackOffSpec` 
deserialize to the field default `"default"`, which matches the pre-PR behavior 
(the attribute was previously null → `Optional.orElse(getDefault())` → 
`DefaultRetryStrategy`).
   
   One minor inconsistency: the SQL scripts add a `custom` dict entry 
(`'custom','custom','custom',3,0`, `enabled=0`) across all dialects, but 
`HttpRetryBackoffSpecEnum` no longer has `CUSTOM_BACKOFF` (removed in this PR). 
So `custom` is now a dead/disabled dict value — `acquireByName("custom")` falls 
through to `DEFAULT_BACKOFF`. Since the entry is `enabled=0` it won't surface 
in the admin UI, so there's no runtime impact, but you could either drop the 
`custom` dict rows or keep the enum value to avoid the mismatch. Just flagging.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to