SEZ9 commented on PR #12293: URL: https://github.com/apache/seatunnel/pull/12293#issuecomment-5862359644
Thanks @sudeephazra for the update on 904107e329c94cbd177b821246327b3ef2788487. **Issue 1** — thanks for restoring the option-key context via catch-and-rewrap in `ADLSConfigValidator.validate`. I'll re-check the OAuth validation messages on this head to confirm. **Issue 2** — understood that a native-speaker review of the Chinese docs is hard to arrange; fine to leave that open for now. Separately, these points from the earlier review still look open to me — please let me know if any have already been addressed: - The validator trims option values but `ADLSHadoopConf` consumes the raw values, so surrounding whitespace (e.g. from env-var substitution) can pass validation and then fail later with an unmapped exception or a worker-side 403. Please normalize once and pass the trimmed values through, or trim again in `ADLSHadoopConf`. - Sink docs: add the HNS (hierarchical namespace) requirement alongside the exactly-once / transactional-commit claim, since that is what makes the `tmp_path` → `path` rename atomic. - Document the validation rules that reject configs (account_name / container naming, auth mutual exclusion, blocked `hadoop_adls_properties` keys) and update the shipped template placeholders so they pass those rules. - Validate `authority_host` / `tenant_id` before building the OAuth token endpoint (require https and a well-formed host) so `client_secret` can't be POSTed over plain HTTP or to an arbitrary URL. - Option tables: make the en and zh sink tables list the same options and match what the factories actually expose, and fix the en source doc defaults for `discovery_mode` and `start_mode` (code defaults are `ONCE` and `EARLIEST`; the zh doc already says so). These don't need a native speaker. - Mention the `ConfigShadeUtils` change adding `account_key` to the default log-mask list in the docs and the PR description, since it affects all connectors. - Tighten the `hadoop_adls_properties` denylist so class-loading and token-provider ABFS keys are blocked too, matching the "non-routing, non-credential" wording on the option. Happy to do a final pass once these land. <!-- streview-comment:1367 --> -- 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]
