DanielLeens commented on PR #12048:
URL: https://github.com/apache/seatunnel/pull/12048#issuecomment-5852831894
Thanks for checking the diff scope on `56794bc19d3e`, @SEZ9 — you're right
that it's config-only and touches no production source, so it doesn't change
any of F1–F8 on its own.
That said, I don't think F1–F8 are actually still open on the code side. In
my re-review at the same head (`56794bc19d3e`, review `#5325863940` above,
posted 2026-09-26), I re-walked the current source from scratch — not the
author's description — and each one resolves to a concrete location:
- **F1** (retention-trimmed checkpoint): documented in
`docs/en/connectors/source/AzureEventHubs.md`, "Partition And Recovery
Semantics" — full paragraph on stopping the job and starting fresh without
restoring old state. This matches the alternative you proposed yourself ("add
an opt-in reset, **or** document the concrete recovery procedure").
- **F2** (`ServicesResourceTransformer`): root `pom.xml:829-876` declares it
under `default-shade` in `<pluginManagement>`;
`connector-azure-event-hubs/pom.xml:63-100` reuses the same execution id with
no `<transformers>` override, so Maven merges the parent transformer in
untouched.
- **F3** (no SAS key on any output path):
`AzureEventHubsSourceConfigTest.java:41-66`
(`entityPathRejectionAndStringRepresentationsDoNotExposeCredentials`) asserts
the full connection string is absent from `config.toString()`,
`AzureEventHubsSourceSplit#toString()`, and both `exception.getMessage()` and
`ExceptionUtils.getMessage(exception)` across two hub values.
- **F4** (unpinned transitive shaded versions): still open, but as a Low,
non-blocking, disclosed risk — I only asked that the follow-up tracking (e.g. a
CVE-check ticket reference) be made explicit rather than just disclosed once.
- **F5** (EntityPath rejection): documented with the least-privilege
rationale in the same doc's `connection_string` section.
- **F6** (`prefetch_count >= max_batch_size` at submission, not just at
reader-open): `AzureEventHubsSourceFactory.optionRule()` (`:47-49`) uses the
*same* builder as the reader-side `from()` gate
(`AzureEventHubsSourceConfig.java:84-110`, `Conditions.greaterOrEqualField` at
`:106-109`), and `AzureEventHubsSourceConfigTest.java:192-206` exercises it
through `ConfigValidator.of(...).validate(new
AzureEventHubsSourceFactory().optionRule())` — the factory rule object itself,
so this fails at job submission, not only at reader open.
- **F7** (no dynamic partition rediscovery): documented, and the actual code
gate is `AzureEventHubsSourceSplitEnumerator.java:67` (`if (!restored)` skips
discovery entirely on restore).
- **F8** (retry/reconnect tuning): documented under "Retry And Failure
Behavior" in the same doc.
Separately, on the config-shade/decrypt concern from my own earlier
`765fb71b0` round (the one High I raised myself, not one of your F-numbers):
still fixed and unchanged at this head. `connection_string` lives only in
`DEFAULT_LOG_MASK_ONLY_KEYWORDS` (`ConfigShadeUtils.java:58-64`), never in
`DEFAULT_SENSITIVE_KEYWORDS` (`:51-52`), so the actual encrypt/decrypt path
(`getSensitiveOptions()`, used at `:186`) doesn't touch it and existing
`AzureQueueStorage` jobs see zero behavior change.
So from my side: no High or Medium remains, F4 is the one open Low, and
F1/F5/F7/F8 are deliberate documented-limitation decisions rather than gaps —
which matches what your own `5788235498` comment on 2026-09-23 had already
accepted for F5/F7/F8. If you still see a gap in any of these against the
current head, a specific line or test that contradicts the citations above
would help close it quickly; otherwise I'd suggest treating
F1/F2/F3/F5/F6/F7/F8 as resolved and F4 as the only remaining (non-blocking)
item.
--
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]
