surafel58 commented on PR #11778: URL: https://github.com/apache/seatunnel/pull/11778#issuecomment-5292879550
Thanks for the detailed re-review @DanielLeens. Pushed e88a0034 with all three non-blocking nits addressed: - Issue 1 (close() exception masking): reworked close() to keep the flush failure as the primary exception and attach any cleanup failures with addSuppressed, so a client-close IOException can no longer replace the 'Writing records to prometheus failed' message. Added a unit test (closeShouldKeepFlushExceptionWhenHttpClientCloseAlsoThrows) that fails the flush and the client close together and asserts the flush exception surfaces with the IOException suppressed. - Issue 2 (stale test comment): reworded the batch_size comment in PrometheusWriterTest to state the real mechanism (unset batch_size now resolves to the default 1024 from #11797, and each test writes a single row well below it). - Issue 3 (warning wording): changed 'one-time warning' to 'once per sink writer at startup' in the EN and ZH incompatible-changes entries, and hoisted the removed 'flush_interval' key into a named constant. Issue 4 (expected conflict with #11738): noted, I will resolve that deliberately whichever of the two lands second, keeping this PR's flush_interval removals and #11738's batch_size lines both intact. Connector module tests pass locally (9 tests). This push will re-run CI. -- 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]
