bharathgunapati commented on PR #54:
URL: 
https://github.com/apache/flink-connector-http/pull/54#issuecomment-5718429727

   > another AI review
   
   1. Retry.of() / global registry — Not a correctness issue on Resilience4j 
1.7.1. Retry.of(name, config) constructs a new RetryImpl; it does not look up 
or reuse a global registry entry. Registry behaviour is a different API 
(RetryRegistry). Creating a Retry per send() is a bit more allocation, but it 
lets retryOnResult close over that attempt’s accumulator. No change planned 
here.
   
   2. Fatal failures now fail the job — Confirmed. Fatal / exhausted paths now 
call completeExceptionally instead of silently completing. That is intentional 
for this PR; jobs that previously counted and continued on http.sink.error.code 
can break. I’ll call this out in the PR compatibility note. Continue-on-error / 
optional error-DLQ is tracked as 
[FLINK-40699](https://issues.apache.org/jira/browse/FLINK-40699).
   
   3. Accumulator on the final successful attempt — Already covered. 
Resilience4j invokes retryOnResult for every completed result, including the 
last one. HttpSinkClientWithRetryTest#testRetriesOnlyRetryableRequests asserts 
that after a retryable entry later succeeds, both entries are in the final 
successful list.
   
   4. Legacy warning vs ignored codes — The warning no longer claims that new 
options are fully disabled when legacy props are set. It only notes that legacy 
error-code properties remain supported and points to the new options for new 
jobs. In legacy mode, exclude still maps into the ignored set and is applied 
first. Mixing legacy and new status-code options now fails at sink creation.
   
   5. Lombok getReadableConfig() — Not a conflict. Lombok skips generating a 
getter when the method already exists; the hand-written method returns 
ReadableConfig. Same pattern as HttpLookupConfig.
   
   6. Manual fromDataStream mapping — Agree it’s repetitive. Tracked typed 
DataStream configuration (and cleaner mapping) as 
[FLINK-40700](https://issues.apache.org/jira/browse/FLINK-40700) under 
FLINK-40275 rather than expanding this PR.


-- 
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