SEZ9 commented on PR #12293: URL: https://github.com/apache/seatunnel/pull/12293#issuecomment-6432417273
Thanks @sudeephazra for the follow-up on `b0bc6fc24aa3`. **CI coverage** – Agreed: keeping default CI credential-free on WASB/Azurite and documenting how to enable the opt-in live ABFS/Gen2 `ADLSFileIT` is the right split. No further ask there. **PR12293-F1 (whitespace normalization)** – Thanks for trimming the `hadoop_adls_properties` values in `ADLSHadoopConf`. One remaining gap in the same family: the trim currently covers the value side only, so a key with surrounding whitespace (easy to get via env-var substitution) still goes into the Hadoop config un-normalized and silently becomes a no-op property. Could you trim the key as well (and reject or ignore keys that are empty after trimming) so validation and consumption see the same normalized key/value pair? A small unit test with a padded key would be great. **Remaining previous findings** – Comparing against `92e5a6094d5a`, I couldn't confirm a change for the items below. Could you either address them or point me to where they were handled? - **F2** – `docs/en/connectors/sink/ADLSFile.md`: add the HNS (hierarchical namespace) requirement for the atomic `tmp_path` -> `path` rename behind the exactly-once/transactional claim. - **F3** – document the validation rules (account_name/container naming, auth mutual exclusion, blocked `hadoop_adls_properties` keys) and fix the shipped template placeholders so they pass those rules. - **F4** – `ADLSRuntimeCompatibility.java`: validate `authority_host`/`tenant_id` before building the OAuth token endpoint so `client_secret` can't be POSTed over plain HTTP or to an arbitrary URL. - **F5** – reconcile the en/zh sink option tables with each other and with what the factories actually expose. - **F6** – mention the `ConfigShadeUtils` change (adding `account_key` to the default log-mask list) in the docs and PR description, since it's a global core-starter behaviour change. - **F7** – `docs/en/connectors/source/ADLSFile.md`: fix the `discovery_mode` / `start_mode` defaults to `ONCE` / `EARLIEST` to match the code and the Chinese doc. - **F8** – `ADLSConfigValidator.java`: tighten the `hadoop_adls_properties` denylist to also cover the class-loading and token-provider ABFS keys, or soften the "non-routing, non-credential" wording on the option to match what is actually enforced. Once the key-side trim and the items above are in (or you point me to where they already are), I'm happy to take another pass. <!-- streview-comment:2071 --> -- 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]
