featzhang commented on PR #27561: URL: https://github.com/apache/flink/pull/27561#issuecomment-4293222443
Thank you for the review! Let me address your comments: **Comment 1 - Duplicate if/else bodies**: Looking at the code structure, the client error handling (4xx) and unexpected status code handling have different behaviors: - Client errors (4xx): Non-retryable, returns default value if configured, otherwise fails immediately - Unexpected status codes: Retryable via `handleFailureWithRetry` While both have an else branch with `future.completeExceptionally(exception)`, they serve different purposes in different contexts. The 4xx case fails immediately after trying the default value, while the unexpected case only reaches that point after retries are exhausted. Could you clarify which specific lines you're referring to? **Comment 2 - Error not passed back at line 278**: This is intentional behavior. When a default value is configured and all retries are exhausted, the function returns the default value instead of propagating the error. This allows graceful degradation - the pipeline continues processing with fallback data rather than failing. The original error is still logged at WARN level (line 271-275) for observability. If we passed the error back when returning the default value, it would defeat the purpose of having a fallback mechanism. The current design allows users to choose between: 1. Fail-fast (no default value configured): Error propagates to Flink 2. Graceful degradation (default value configured): Default returned, error only logged Does this design make sense, or would you prefer a different approach? -- 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]
