SEZ9 commented on PR #10680: URL: https://github.com/apache/seatunnel/pull/10680#issuecomment-5381236781
Hi @DanielLeens, thanks for the detailed follow-up on the docs question — that answers it fully. Since `0570563e5b` only changes the internal label-state verification/retry logic in `StarRocksSinkManager.java` and `StarRocksStreamLoadVisitor.java` (plus their tests) and doesn't add, remove, or modify any user-facing `Option`/config field, I agree there's no docs update needed for this revision. If a follow-up ever adds a tunable like a configurable max-retry count, we'll revisit docs at that point, as you suggested. I also appreciate the thorough coverage you described for the label-state paths (VISIBLE, COMMITTED, ABORTED, UNKNOWN, Label Already Exists, retry exhaustion, retained batches, interruption) — that's exactly the safety net this data-loss fix needed. Remaining items before this can merge: 1. The `Build` run is still in `ACTION_REQUIRED` (https://github.com/apache/seatunnel/runs/88075956675), same as the earlier check (https://github.com/apache/seatunnel/pull/10680/checks?check_run_id=69335206933). We need the workflows enabled/retriggered and a green `Build` result. 2. As you noted, your `COMMENTED` review records the technical conclusion but doesn't satisfy the approval gate, so a write-capable maintainer will need to complete the formal approval once CI is green. Once CI passes I'll take a final look and handle the approval. Thanks again for picking this up and driving it to completion. <!-- streview-comment:447 --> -- 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]
