SEZ9 commented on PR #12048: URL: https://github.com/apache/seatunnel/pull/12048#issuecomment-5825873934
Thanks for the detailed re-review of `1aa4ee993`. **F6 / F3 / F2 — need diff confirmation before closing** The explanation is helpful, but I'd like to verify these against the actual changes rather than close them on description alone. Could you point me to the changed lines in `1aa4ee993` (or anything since `3e4ee6d3320e`) for: - **F6** — the `factoryRulesRejectBlankAndCrossFieldOptions` test validating `prefetch_count >= max_batch_size` through the factory's own `OptionRule`. - **F3** — the assertion that the full `CONNECTION_STRING` (not only the bare `SAS_KEY`) is absent from `config.toString()` and `AzureEventHubsSourceSplit#toString()`. - **F2** — the connector pom's shade execution and where the root `ServicesResourceTransformer` configuration is inherited from. A sentence in the PR description recording this would also help prevent someone later adding a `<transformers>` block to the connector pom that overrides it. **Still open — please reply on each** - **F1 (retention-trimmed position → restart crash-loop)** — failing the task on a retention gap is a reasonable default, but I'd still like an operator escape hatch. Please either add an explicit opt-in (e.g. reset to earliest/latest when the checkpointed sequence number is no longer available) or document the concrete recovery procedure in `docs/en/connectors/source/AzureEventHubs.md`, and point me to where it lands. - **F4 (unpinned transitive Netty/Jackson/Proton in the shaded jar)** — please either pin these via the root dependency management or explain in the PR description why the Azure SDK-resolved versions are acceptable and how they will be tracked for CVE remediation. - **F5 (rejecting `EntityPath` connection strings blocks hub-scoped SAS policies)** — please either accept an `EntityPath`-bearing connection string when it matches the configured hub, or document why the rejection is required. - **F7 (no partition rediscovery)** — please confirm whether this is intentionally out of scope; if so, document the limitation and state that a partition-count increase requires a job restart. - **F8 (no connector-level reconnect/retry tuning)** — please either expose the SDK retry options or document the default retry window and its failure behaviour. If I've missed updates on any of these, just point me to them. Once F1 has a concrete answer and the rest each have either a change or a documented decision, I'm happy to approve. <!-- streview-comment:1300 --> -- 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]
