SEZ9 commented on PR #12048: URL: https://github.com/apache/seatunnel/pull/12048#issuecomment-5754791971
Thanks for `765fb71b0` and for the point-by-point on F1–F8, @goutamadwant. Treating reset, rediscovery and retry tuning as documented scope boundaries rather than adding new controls is acceptable for a first version, as long as the docs say so plainly. Where things stand from my side: - **F1 (retention gap)** — Fail-not-reset is the right default. What I still need is the operator story in the connector doc: when the checkpointed position has been trimmed, what does the user concretely do to get the job running again? Please spell that out (even if the answer is "restart the job without restoring state"), so the failure doesn't read as a dead end. - **F2 (ServicesResourceTransformer)** — Good that the isolated package probe loads the factory and the Jackson providers. Please point me at where the service-resource merging is inherited from so I can verify it applies to this module's shade execution, and confirm the probe ran against the packaged connector jar rather than the build classpath. - **F3 (connection-string masking)** — Parsed-config masking covering `connection_string` addresses one path. Please confirm the other two from the finding: the error message produced when EntityPath handling rejects or mismatches, and the split/config `toString` output, neither of which should carry the SAS key. - **F4 (transitive versions)** — Please point me at the dependency management in the diff and confirm the Netty/Jackson/Proton versions that end up in the shaded jar are the root-managed ones, not whatever the Azure SDK pulls transitively. - **F5 (EntityPath)** — Converting instead of rejecting sounds like the right direction for the least-privilege concern. Please point me at the conversion code so I can verify the behavior, and make sure the connector doc describes it. - **F6 (prefetch_count >= max_batch_size)** — Please point me at the factory-time validation in the diff and confirm one of the 42 tests covers the rejection path at the factory. - **F7 / F8 (rediscovery, retry tuning)** — Accepting these as limitations. Please make sure the doc states the consequence of each: partitions added at runtime are not read until the job is restarted, and outages longer than the SDK retry window surface as a task failure handled by job-level recovery. Live Azure authorization and outage recovery being unverified is understood; a note in the PR description to that effect is enough. I'll re-review F1–F8 once the doc updates and the pointers above are in. <!-- streview-comment:1205 --> -- 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]
