SEZ9 commented on PR #12293: URL: https://github.com/apache/seatunnel/pull/12293#issuecomment-5944655975
Thanks for the pointers on 781ba2ec. Going through them in order: - **F1**: Thanks for the `configuresOAuthClientCredentials()` reference in `ADLSHadoopConfTest`. The core of this finding was the trim/normalize mismatch: the validator checks trimmed values while `ADLSHadoopConf` consumes the raw values, so a whitespace-padded value from env-var substitution can pass validation and fail later. Could you confirm whether `ADLSHadoopConf` now reads the normalized values (or the validator rejects untrimmed input), and whether any test exercises a padded value? - **F2**: The HNS prerequisite, same-container requirement and partial-output explanation you describe for the English and Chinese sink docs is exactly what was missing. I'll verify the wording in the diff before closing this one. - **F3**: Documenting naming rules, auth mutual exclusion and blocked Hadoop properties in both languages (mirrored in the source docs), plus naming-valid template placeholders, addresses the concern. Requiring the account key from an environment value is fine. I'll confirm against the diff. - **F4**: The described checks in `ADLSRuntimeCompatibility` (HTTPS-only authority with a parseable host, rejecting userinfo/path/query/fragment, GUID-or-DNS tenant) address the concern. Is there a unit test asserting that a non-HTTPS authority and a non-GUID/non-DNS tenant are rejected before the endpoint is built? A pointer would be enough, and I'll check the implementation in the diff. - **F5**: Your comment appears to have been cut off right after the English sink table link, so I can't see the rest of the F5 response or anything after it. Could you re-post the remainder? Since the truncation also hides anything you may have written for **F6** (the `ConfigShadeUtils` default mask list change being called out in docs/PR description), **F7** (English source doc defaults for `discovery_mode` / `start_mode` matching the code and the Chinese doc) and **F8** (the `hadoop_adls_properties` denylist versus the documented non-routing/non-credential contract), pointers for those would let me wrap up the re-review in one pass. <!-- streview-comment:1459 --> -- 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]
