SEZ9 commented on PR #12048: URL: https://github.com/apache/seatunnel/pull/12048#issuecomment-5852151222
Thanks for the update. Since `1aa4ee993`, the only new commit is `56794bc19d3e`, which changes the E2E job config (`azure_event_hubs_to_console.conf`, adding `plugin_output`/`plugin_input` wiring for the two-sink job) and no production source. That fix looks right on its own, but it means the previously raised findings are unchanged on the code side, so they remain open from my perspective: - **F1 (HIGH, restart crash-loop on retention-trimmed checkpoint position):** please provide an operator escape hatch (e.g. an option to fall back to earliest/latest when the checkpointed offset is no longer available) rather than a permanent failure on restart, and describe the behavior in `docs/en/connectors/source/AzureEventHubs.md`. - **F2 (shade relocations without `ServicesResourceTransformer`):** relocating `com.azure` / `io.netty` / `org.apache.qpid.proton` in `connector-azure-event-hubs/pom.xml` can break Azure SDK ServiceLoader providers in the packaged jar unless the transformer is added. Please add it, or share evidence from a run against the packaged jar (not the IDE classpath). - **F3 (connection-string masking on every output path):** please confirm the option is flagged sensitive, the EntityPath-rejection error message does not echo the raw string, and split/config `toString` never includes the SAS key. - **F4 (unpinned shaded transitive versions):** please pin or explicitly manage the Netty/Jackson/Proton versions that end up inside the shaded jar so they remain visible to SeaTunnel's dependency management. - **F5 (rejecting `EntityPath` connection strings):** this blocks hub-scoped, least-privilege SAS policies. Consider accepting `EntityPath` (validating it against the configured hub name if both are given) instead of rejecting it. - **F6 (`prefetch_count >= max_batch_size`):** this cross-field check should fail at job submission in config validation, not at reader open on the worker. - **F7 (no partition rediscovery):** at minimum the docs should state plainly that increasing the Event Hub partition count at runtime requires a job restart, otherwise new partitions are silently never read. - **F8 (no reconnect/retry tuning):** please expose (or document the defaults for) the SDK retry window so a transient AMQP outage does not fail the whole task with no tuning available. If any of these are already handled, please point me to the exact code locations so I can confirm. Once F1 and F2 are addressed and the rest are either fixed or explicitly documented as limitations, I'm happy to take another pass. <!-- streview-comment:1359 --> -- 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]
