SEZ9 commented on PR #12048: URL: https://github.com/apache/seatunnel/pull/12048#issuecomment-5747014449
Thanks @goutamadwant for `86129f57f`. The description matches what I want on the two points that matter most: payload text no longer reaches the failure trace, and a failed event neither gets skipped nor advances the checkpointed position. Good that the regression was reproduced before the fix, and noted that the 41 connector tests and 4 Markdown tests are local parser/emitter verification rather than a new live Azure run. One follow-up on that commit, tied to the "masking must hold on every output path" point: the catch introduced there looks broader than it needs to be, and for non-payload failures the message is reduced to `Could not deserialize or emit Event Hubs event at sequence number N` with no partition id. Could you narrow the catch to the deserialize/emit path and add the partition id to the message (still keeping raw event text and the connection string out of it)? That keeps the trace safe without stripping the operator's diagnostic context. For the rest of the earlier review, I don't see status in the thread yet, so please confirm where each stands at `86129f57`: 1. **Retention-trimmed position → crash-loop (HIGH):** is there now an operator escape hatch (e.g. an explicit reset option) so a restart can recover, and is it documented in `docs/en/connectors/source/AzureEventHubs.md`? 2. **Shade relocations of `com.azure` / `io.netty` / `org.apache.qpid.proton`:** has a `ServicesResourceTransformer` been added in `seatunnel-connectors-v2/connector-azure-event-hubs/pom.xml`, and have you verified the packaged jar's ServiceLoader providers resolve at runtime? 3. **Connection-string masking on every path:** option sensitivity flag, the EntityPath-rejection error message, and split/config `toString` — please confirm each has been checked with a live-looking SAS key. 4. **Unpinned transitive Netty/Jackson/Proton versions in the shaded jar:** are these now managed explicitly so they stay visible to dependency management? 5. **EntityPath rejection:** hub-scoped SAS policies are a legitimate least-privilege setup; can EntityPath be accepted (validated against the configured hub) instead of rejected? 6. **`prefetch_count >= max_batch_size`:** does this now fail at job submission rather than at reader open on the worker? 7. **Partition rediscovery:** how are new partitions handled when the count is increased at runtime — periodic rediscovery, or at minimum a documented limitation with a fail-loud check? 8. **Reconnect/retry tuning:** is there a connector-level retry/reconnect option, or documented behavior for AMQP outages longer than the SDK default window? Short "done in commit X / not yet / disagree because …" per item is enough. Thanks again for the steady work on this connector. <!-- streview-comment:1175 --> -- 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]
