whua3 commented on code in PR #11713:
URL: https://github.com/apache/gravitino/pull/11713#discussion_r3437231001
##########
catalogs/hadoop-common/src/main/java/org/apache/gravitino/catalog/hadoop/fs/Constants.java:
##########
@@ -67,6 +67,10 @@ public class Constants {
public static final String OSS_ESTABLISH_TIMEOUT_KEY =
"fs.oss.connection.establish.timeout";
public static final String OSS_MAX_ERROR_RETRIES_KEY =
"fs.oss.attempts.maximum";
+ // Tencent Cloud COS (hadoop-cos / CosNFileSystem) specific configuration
keys
+ public static final String COS_CONNECTION_TIMEOUT_KEY =
"fs.cosn.connection.timeout";
Review Comment:
Thanks for the careful review. I just verified against
`hadoop-cos:3.3.0-8.3.23` by decompiling `CosNConfigKeys.class` from the
published Maven Central artifact. Both keys are real:
- `fs.cosn.connection.timeout` → `CosNConfigKeys.COSN_CONNECTION_TIMEOUT`.
Upstream default is `10000` ms.
- `fs.cosn.maxRetries` → `CosNConfigKeys.COSN_MAX_RETRIES_KEY`. Upstream
default is `200`.
So the defaults injected by `additionalCOSConfig` are **not** silently
ignored.
Regarding *"prefer referencing `CosNConfigKeys.*` constants over hardcoded
literals"* — I followed the existing convention used by `S3FileSystemProvider`,
`OSSFileSystemProvider` and `GCSFileSystemProvider`: each defines its
cloud-specific keys as **string literals** in `hadoop-common/Constants.java`,
so `hadoop-common` does not depend on any cloud-vendor SDK. Switching just COS
to use `CosNConfigKeys.*` would break that consistency.
I added Javadoc to the COS keys in `Constants.java` cross-referencing the
corresponding `CosNConfigKeys` constants and their upstream defaults, to make
the mapping easy to verify going forward.
--
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]
