sudeephazra commented on PR #12293: URL: https://github.com/apache/seatunnel/pull/12293#issuecomment-6435696436
@DanielLeens @SEZ9 Thanks a lot for the feedback. Please see the observations below @DanielLeens Thanks for the review. Both recommended fixes are implemented: - Issue 1: ADLSHadoopConfTest.preservesAdvancedPropertiesWithoutChangingTheAdlsFileSystem() supplies " true " and asserts "true" (lines 123 and 135). Lines 124–138 also cover a padded key/value pair. - Issue 2: ADLSHadoopConf.java:71 now uses options.put(key.trim(), value.trim()). ADLSConfigValidator.java:100 rejects keys empty after trimming, with coverage in ADLSConfigValidatorTest.rejectsBlankAdvancedPropertyKeys(). English and Chinese docs match this behavior. @SEZ9 For your comments - CI coverage - Fixed already - PR12293-F1 (whitespace normalization) – Keys and values are both trimmed: options.put(key.trim(), value.trim()). Existing validation rejects keys that are empty after trimming. Unit coverage now includes a padded key/value pair and empty or whitespace-only keys. English and Chinese docs reflect this behavior. - F1 / recommended fixes 1–2: Both keys and values are trimmed in [ADLSHadoopConf.java:71](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/seatunnel-connectors-v2/connector-file/connector-file-adls/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/adls/config/ADLSHadoopConf.java#L71). Padded-value/key assertions are in [ADLSHadoopConfTest.java:119](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/seatunnel-connectors-v2/connector-file/connector-file-adls/src/test/java/org/apache/seatunnel/connectors/seatunnel/file/adls/config/ADLSHadoopConfTest.java#L119); blank-key coverage is in ADLSConfigValidatorTest.rejectsBlankAdvancedPropertyKeys(). F2: HNS and same-container rename prerequisites: [English sink:39](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/docs/en/connectors/sink/ADLSFile.md#L39). F3: Naming, authentication exclusion, and blocked-property rules: [English sink:100](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/docs/en/connectors/sink/ADLSFile.md#L100), mirrored across all four references. Valid placeholders: [template:39](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/config/v2.batch.adls.config.template#L39). F4: Pre-construction authority/tenant validation: [ADLSRuntimeCompatibility.java:125](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/seatunnel-connectors-v2/connector-file/connector-file-adls-runtime/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/adls/config/ADLSRuntimeCompatibility.java#L125). Custom HTTPS authorities remain accepted for compatibility; there is no host allowlist. Trusted operators must select the credential recipient. Limitation recorded in [PR_DESCRIPTION.md:39](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/PR_DESCRIPTION.md#L39). F5: Factory-aligned sink tables: [English:52](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/docs/en/connectors/sink/ADLSFile.md#L52), [Chinese:37](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/docs/zh/connectors/sink/ADLSFile.md#L37). F6: Global account_key log masking, distinct from encryption: [sink docs:133](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/docs/en/connectors/sink/ADLSFile.md#L133) and [PR notes:60](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/PR_DESCRIPTION.md#L60). F7: ONCE / EARLIEST defaults: [English source:74](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/docs/en/connectors/source/ADLSFile.md#L74). F8: Expanded denylist: [ADLSConfigValidator.java:97](https://github.com/sudeephazra/seatunnel-adls-connector-v2/blob/ADLS_MergeConflict/seatunnel-connectors-v2/connector-file/connector-file-adls/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/adls/config/ADLSConfigValidator.java#L97). Docs explicitly describe its non-exhaustive, trusted-input scope. -- 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]
