SEZ9 commented on PR #12293: URL: https://github.com/apache/seatunnel/pull/12293#issuecomment-5902776391
@sudeephazra thanks for the detailed rundown — this covers most of the earlier review points. Acknowledged as addressed based on your description: - Validation rules documented under "ADLS configuration rules" in all four en/zh pages, with template placeholders (`replacewithaccount` / `replacewithcontainer`) now satisfying the naming rules. - `tenant_id` / `authority_host` validated before the token endpoint is assembled (HTTPS origin only, GUID or DNS tenant), with validator/runtime/Hadoop-conf tests. - Option tables covering the factory-registered options and en/zh sink keys aligned. - `account_key` added to `ConfigShadeUtils.DEFAULT_LOG_MASK_ONLY_KEYWORDS`, with docs clarifying this is global log masking rather than config encryption. - English source defaults corrected to `discovery_mode = ONCE` and `start_mode = EARLIEST`. - `hadoop_adls_properties` denylist extended to token-provider and class-loading prefixes, covered by `ADLSConfigValidatorTest`. Two items from the previous review are not mentioned in your summary, so I'd like to confirm their status: 1. **Trimmed vs. raw values** — the validator normalizes (trims) option values, but `ADLSHadoopConf` was reading the raw values, so whitespace coming in via env-var substitution could pass validation and then fail later with an un-mapped `IllegalArgumentException` or a worker-side 403. Is `ADLSHadoopConf` now consuming the same normalized values the validator checks (or is the trimming applied once upstream of both)? If so, a short pointer to the change and a test covering a padded value would be great. 2. **HNS requirement in sink docs** — the sink docs describe exactly-once / transactional commit, which relies on the `tmp_path` -> `path` rename being atomic, and that in turn requires a hierarchical-namespace (HNS) enabled ADLS Gen2 account. Has a note about this prerequisite been added to `docs/en/connectors/sink/ADLSFile.md` (and the zh counterpart)? If not, please add it alongside the transactional-commit description. Once those two are confirmed or updated, I'm happy to take another pass. <!-- streview-comment:1412 --> -- 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]
