Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao merged PR #5372: URL: https://github.com/apache/gravitino/pull/5372 -- 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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 closed pull request #5372: [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. URL: https://github.com/apache/gravitino/pull/5372 -- 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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao merged PR #5244: URL: https://github.com/apache/gravitino/pull/5244 -- 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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820847479
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java:
##
@@ -385,13 +383,14 @@ private FilesetContextPair getFilesetContext(Path
virtualPath, FilesetDataOperat
scheme,
str -> {
try {
-Map maps = getConfigMap(getConf());
FileSystemProvider provider =
fileSystemProvidersMap.get(scheme);
if (provider == null) {
throw new GravitinoRuntimeException(
"Unsupported file system scheme: %s for %s.",
scheme,
GravitinoVirtualFileSystemConfiguration.GVFS_SCHEME);
}
+
+Map maps = getConfigMap(getConf());
Review Comment:
What is the gvfs configuration for now, is it aligned with server side conf,
or still use "."?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820871800
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java:
##
@@ -385,13 +383,14 @@ private FilesetContextPair getFilesetContext(Path
virtualPath, FilesetDataOperat
scheme,
str -> {
try {
-Map maps = getConfigMap(getConf());
FileSystemProvider provider =
fileSystemProvidersMap.get(scheme);
if (provider == null) {
throw new GravitinoRuntimeException(
"Unsupported file system scheme: %s for %s.",
scheme,
GravitinoVirtualFileSystemConfiguration.GVFS_SCHEME);
}
+
+Map maps = getConfigMap(getConf());
Review Comment:
Have you verified manually using gvfs?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on PR #5244: URL: https://github.com/apache/gravitino/pull/5244#issuecomment-239118 Overall LGTM, please fix the ci issue. -- 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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820849177
##
bundles/aliyun-bundle/src/main/java/org/apache/gravitino/oss/fs/OSSFileSystemProvider.java:
##
@@ -18,23 +18,41 @@
*/
package org.apache.gravitino.oss.fs;
+import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Map;
import org.apache.gravitino.catalog.hadoop.fs.FileSystemProvider;
+import org.apache.gravitino.storage.OSSProperties;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.aliyun.oss.AliyunOSSFileSystem;
+import org.apache.hadoop.fs.aliyun.oss.Constants;
public class OSSFileSystemProvider implements FileSystemProvider {
+
+ private static final String OSS_FILESYSTEM_IMPL = "fs.oss.impl";
+
+ public static final Map GRAVITINO_KEY_TO_OSS_HADOOP_KEY =
Review Comment:
Again, why do we turn it back to `public`?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820879442
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java:
##
@@ -385,13 +383,14 @@ private FilesetContextPair getFilesetContext(Path
virtualPath, FilesetDataOperat
scheme,
str -> {
try {
-Map maps = getConfigMap(getConf());
FileSystemProvider provider =
fileSystemProvidersMap.get(scheme);
if (provider == null) {
throw new GravitinoRuntimeException(
"Unsupported file system scheme: %s for %s.",
scheme,
GravitinoVirtualFileSystemConfiguration.GVFS_SCHEME);
}
+
+Map maps = getConfigMap(getConf());
Review Comment:
Yes , `GravitinoVirtualFileSystemS3IT` can be running in the CI and cover
the changes.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820870977
##
bundles/aliyun-bundle/src/main/java/org/apache/gravitino/oss/fs/OSSFileSystemProvider.java:
##
@@ -18,23 +18,41 @@
*/
package org.apache.gravitino.oss.fs;
+import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Map;
import org.apache.gravitino.catalog.hadoop.fs.FileSystemProvider;
+import org.apache.gravitino.storage.OSSProperties;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.aliyun.oss.AliyunOSSFileSystem;
+import org.apache.hadoop.fs.aliyun.oss.Constants;
public class OSSFileSystemProvider implements FileSystemProvider {
+
+ private static final String OSS_FILESYSTEM_IMPL = "fs.oss.impl";
+
+ public static final Map GRAVITINO_KEY_TO_OSS_HADOOP_KEY =
Review Comment:
This is because you don't put IT and code together, so you have to make it
public, can you please add a visible for test tag on it?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820855639
##
bundles/aliyun-bundle/src/main/java/org/apache/gravitino/oss/fs/OSSFileSystemProvider.java:
##
@@ -18,23 +18,41 @@
*/
package org.apache.gravitino.oss.fs;
+import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Map;
import org.apache.gravitino.catalog.hadoop.fs.FileSystemProvider;
+import org.apache.gravitino.storage.OSSProperties;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.aliyun.oss.AliyunOSSFileSystem;
+import org.apache.hadoop.fs.aliyun.oss.Constants;
public class OSSFileSystemProvider implements FileSystemProvider {
+
+ private static final String OSS_FILESYSTEM_IMPL = "fs.oss.impl";
+
+ public static final Map GRAVITINO_KEY_TO_OSS_HADOOP_KEY =
Review Comment:
This value will be used in GravitinoVirtualFileSystemOSSIT test and others
are similar.
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java:
##
@@ -385,13 +383,14 @@ private FilesetContextPair getFilesetContext(Path
virtualPath, FilesetDataOperat
scheme,
str -> {
try {
-Map maps = getConfigMap(getConf());
FileSystemProvider provider =
fileSystemProvidersMap.get(scheme);
if (provider == null) {
throw new GravitinoRuntimeException(
"Unsupported file system scheme: %s for %s.",
scheme,
GravitinoVirtualFileSystemConfiguration.GVFS_SCHEME);
}
+
+Map maps = getConfigMap(getConf());
Review Comment:
They are the same as those in Gravitino server side.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820847479
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystem.java:
##
@@ -385,13 +383,14 @@ private FilesetContextPair getFilesetContext(Path
virtualPath, FilesetDataOperat
scheme,
str -> {
try {
-Map maps = getConfigMap(getConf());
FileSystemProvider provider =
fileSystemProvidersMap.get(scheme);
if (provider == null) {
throw new GravitinoRuntimeException(
"Unsupported file system scheme: %s for %s.",
scheme,
GravitinoVirtualFileSystemConfiguration.GVFS_SCHEME);
}
+
+Map maps = getConfigMap(getConf());
Review Comment:
What is the gvfs configuration for now, is it aligned with server side conf,
or still use "."? I cannot find the code anymore.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820595345
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystemConfiguration.java:
##
@@ -107,5 +112,46 @@ public class GravitinoVirtualFileSystemConfiguration {
public static final long
FS_GRAVITINO_FILESET_CACHE_EVICTION_MILLS_AFTER_ACCESS_DEFAULT =
1000L * 60 * 60;
+ private static final Map GVFS_S3_KEY_TO_HADOOP_KEY =
Review Comment:
The fact that this will make module `hadoop3-filesystem` depend on bundle
jars in the runtime explicitly will hinder me from doing so. If this is not a
big problem, I would like to do so.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820611940
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystemConfiguration.java:
##
@@ -107,5 +112,46 @@ public class GravitinoVirtualFileSystemConfiguration {
public static final long
FS_GRAVITINO_FILESET_CACHE_EVICTION_MILLS_AFTER_ACCESS_DEFAULT =
1000L * 60 * 60;
+ private static final Map GVFS_S3_KEY_TO_HADOOP_KEY =
Review Comment:
Why there is a dependency? If user sets the configurations via Hadoop
`Configuration` in gvfs, when gvfs create a provider, it can pass the configs
from gvfs' `Configuration` to provider's config map. Each provider can use this
config map to check the configurations and initialize the FS.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820492254
##
catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopGCSCatalogIT.java:
##
@@ -106,4 +112,64 @@ protected void createCatalog() {
protected String generateLocation(String filesetName) {
return String.format("%s/%s", defaultBaseLocation, filesetName);
}
+
+ @Test
+ public void testCreateSchemaAndFilesetWithSpecialLocation() {
+String localCatalogName = GravitinoITUtils.genRandomName("local_catalog");
+
+String ossLocation = String.format("gs://%s", BUCKET_NAME);
+Map catalogProps = Maps.newHashMap();
+catalogProps.put("location", ossLocation);
+catalogProps.put(GCS_SERVICE_ACCOUNT_JSON_PATH, SERVICE_ACCOUNT_FILE);
+catalogProps.put(FILESYSTEM_PROVIDERS, "gcs");
+
+Catalog localCatalog =
+metalake.createCatalog(
+localCatalogName, Catalog.Type.FILESET, provider, "comment",
catalogProps);
+Assertions.assertEquals(ossLocation,
localCatalog.properties().get("location"));
+
+// Create schema without specifying location.
+Schema localSchema =
+localCatalog
+.asSchemas()
+.createSchema("local_schema", "comment", ImmutableMap.of("key1",
"val1"));
+
+Fileset localFileset =
+localCatalog
+.asFilesetCatalog()
+.createFileset(
+NameIdentifier.of(localSchema.name(), "local_fileset"),
+"fileset comment",
+Fileset.Type.MANAGED,
+null,
+ImmutableMap.of("k1", "v1"));
+Assertions.assertEquals(
+ossLocation + "/local_schema/local_fileset",
localFileset.storageLocation());
+
+// Delete schema
+localCatalog.asSchemas().dropSchema(localSchema.name(), true);
+
+// Create schema with specifying location.
+Map schemaProps = ImmutableMap.of("location", ossLocation);
+Schema localSchema2 =
+localCatalog.asSchemas().createSchema("local_schema2", "comment",
schemaProps);
+Assertions.assertEquals(ossLocation,
localSchema2.properties().get("location"));
+
+Fileset localFileset2 =
+localCatalog
+.asFilesetCatalog()
+.createFileset(
+NameIdentifier.of(localSchema2.name(), "local_fileset2"),
+"fileset comment",
+Fileset.Type.MANAGED,
+null,
+ImmutableMap.of("k1", "v1"));
+Assertions.assertEquals(ossLocation + "/local_fileset2",
localFileset2.storageLocation());
+
+// Delete schema
+localCatalog.asSchemas().dropSchema(localSchema2.name(), true);
+
+// Delete catalog
+metalake.dropCatalog(localCatalogName, true);
+ }
Review Comment:
Can you add a UT for this?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820586189
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystemConfiguration.java:
##
@@ -107,5 +112,46 @@ public class GravitinoVirtualFileSystemConfiguration {
public static final long
FS_GRAVITINO_FILESET_CACHE_EVICTION_MILLS_AFTER_ACCESS_DEFAULT =
1000L * 60 * 60;
+ private static final Map GVFS_S3_KEY_TO_HADOOP_KEY =
Review Comment:
I still suggest to move this to each provider, we don't have to expose them
here.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820583992
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/fs/FileSystemUtils.java:
##
@@ -101,4 +102,45 @@ public static FileSystemProvider
getFileSystemProviderByName(
"File system provider with name '%s' not found in the
file system provider list.",
fileSystemProviderName)));
}
+
+ /**
+ * Convert the Gravitino configuration to Hadoop configuration.
+ *
+ * Predefined keys have the highest priority. If the key does not exist
in the predefined keys,
+ * it will be set to the configuration. Keys with prefixes
'gravitino.bypass' has the lowest
+ * priority.
+ *
+ * @param config Gravitino configuration
+ * @return Hadoop configuration Map
+ */
+ public static Map toHadoopConfigMap(
+ Map config, Map predefinedKeys) {
+Map result = Maps.newHashMap();
+Set highestPriorityKeys = Sets.newHashSet();
+config.forEach(
+(k, v) -> {
+ if (predefinedKeys.containsKey(k)) {
+String key = predefinedKeys.get(k);
+highestPriorityKeys.add(key);
+result.put(key, v);
+ }
+
+ if (!k.startsWith(GRAVITINO_BYPASS)) {
+// If the key does not start with 'gravitino.bypass' and is not in
the highest priority
+// keys, set the value to the configuration.
+if (!highestPriorityKeys.contains(k)) {
+ result.put(k, v);
+}
+ } else {
+// If the key starts with 'gravitino.bypass', remove the prefix
and set the value to the
+// configuration if the key does not exist in the configuration.
+String key = k.replace(GRAVITINO_BYPASS, "");
+if (!result.containsKey(key)) {
+ result.put(key, v);
+}
+ }
+});
Review Comment:
Can you add a UT for this?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820473016
##
catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopGCSCatalogIT.java:
##
@@ -107,4 +112,64 @@ protected void createCatalog() {
protected String generateLocation(String filesetName) {
return String.format("%s/%s", defaultBaseLocation, filesetName);
}
+
+ @Test
+ public void testCreateSchemaAndFilesetWithSpecialLocation() {
Review Comment:
This is to fix the bug introduced by #5296, we need to consider other Hadoop
catalog like S3, OSS and GCS.
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystemConfiguration.java:
##
@@ -107,5 +112,46 @@ public class GravitinoVirtualFileSystemConfiguration {
public static final long
FS_GRAVITINO_FILESET_CACHE_EVICTION_MILLS_AFTER_ACCESS_DEFAULT =
1000L * 60 * 60;
+ private static final Map GVFS_S3_KEY_TO_HADOOP_KEY =
Review Comment:
In the long run, we can provide a method like `requiredPropertyKey` in the
`FileSystemProvider` and load this provider by service provider. By doing so,
we can register this property to the client dynamically. Currently, we cannot
avoid this hard coding.
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/fs/FileSystemProvider.java:
##
@@ -32,6 +32,8 @@
*/
public interface FileSystemProvider {
+ String GRAVITINO_BYPASS = "gravitino.bypass.";
Review Comment:
added
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820544716
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/fs/FileSystemUtils.java:
##
@@ -101,4 +102,45 @@ public static FileSystemProvider
getFileSystemProviderByName(
"File system provider with name '%s' not found in the
file system provider list.",
fileSystemProviderName)));
}
+
+ /**
+ * Convert the Gravitino configuration to Hadoop configuration.
+ *
+ * Predefined keys have the highest priority. If the key does not exist
in the predefined keys,
+ * it will be set to the configuration. Keys with prefixes
'gravitino.bypass' has the lowest
+ * priority.
+ *
+ * @param config Gravitino configuration
+ * @return Hadoop configuration Map
+ */
+ public static Map toHadoopConfigMap(
+ Map config, Map predefinedKeys) {
+Map result = Maps.newHashMap();
+Set highestPriorityKeys = Sets.newHashSet();
+config.forEach(
+(k, v) -> {
+ if (predefinedKeys.containsKey(k)) {
+String key = predefinedKeys.get(k);
+highestPriorityKeys.add(key);
+result.put(key, v);
+ }
+
+ if (!k.startsWith(GRAVITINO_BYPASS)) {
+// If the key does not start with 'gravitino.bypass' and is not in
the highest priority
+// keys, set the value to the configuration.
+if (!highestPriorityKeys.contains(k)) {
+ result.put(k, v);
+}
+ } else {
+// If the key starts with 'gravitino.bypass', remove the prefix
and set the value to the
+// configuration if the key does not exist in the configuration.
+String key = k.replace(GRAVITINO_BYPASS, "");
+if (!result.containsKey(key)) {
+ result.put(key, v);
+}
+ }
+});
Review Comment:
Can we simplify this code? It's a bit hard to understand, I think it is OK
to iterate for several rounds, just make it easy to read.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820492254
##
catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopGCSCatalogIT.java:
##
@@ -106,4 +112,64 @@ protected void createCatalog() {
protected String generateLocation(String filesetName) {
return String.format("%s/%s", defaultBaseLocation, filesetName);
}
+
+ @Test
+ public void testCreateSchemaAndFilesetWithSpecialLocation() {
+String localCatalogName = GravitinoITUtils.genRandomName("local_catalog");
+
+String ossLocation = String.format("gs://%s", BUCKET_NAME);
+Map catalogProps = Maps.newHashMap();
+catalogProps.put("location", ossLocation);
+catalogProps.put(GCS_SERVICE_ACCOUNT_JSON_PATH, SERVICE_ACCOUNT_FILE);
+catalogProps.put(FILESYSTEM_PROVIDERS, "gcs");
+
+Catalog localCatalog =
+metalake.createCatalog(
+localCatalogName, Catalog.Type.FILESET, provider, "comment",
catalogProps);
+Assertions.assertEquals(ossLocation,
localCatalog.properties().get("location"));
+
+// Create schema without specifying location.
+Schema localSchema =
+localCatalog
+.asSchemas()
+.createSchema("local_schema", "comment", ImmutableMap.of("key1",
"val1"));
+
+Fileset localFileset =
+localCatalog
+.asFilesetCatalog()
+.createFileset(
+NameIdentifier.of(localSchema.name(), "local_fileset"),
+"fileset comment",
+Fileset.Type.MANAGED,
+null,
+ImmutableMap.of("k1", "v1"));
+Assertions.assertEquals(
+ossLocation + "/local_schema/local_fileset",
localFileset.storageLocation());
+
+// Delete schema
+localCatalog.asSchemas().dropSchema(localSchema.name(), true);
+
+// Create schema with specifying location.
+Map schemaProps = ImmutableMap.of("location", ossLocation);
+Schema localSchema2 =
+localCatalog.asSchemas().createSchema("local_schema2", "comment",
schemaProps);
+Assertions.assertEquals(ossLocation,
localSchema2.properties().get("location"));
+
+Fileset localFileset2 =
+localCatalog
+.asFilesetCatalog()
+.createFileset(
+NameIdentifier.of(localSchema2.name(), "local_fileset2"),
+"fileset comment",
+Fileset.Type.MANAGED,
+null,
+ImmutableMap.of("k1", "v1"));
+Assertions.assertEquals(ossLocation + "/local_fileset2",
localFileset2.storageLocation());
+
+// Delete schema
+localCatalog.asSchemas().dropSchema(localSchema2.name(), true);
+
+// Delete catalog
+metalake.dropCatalog(localCatalogName, true);
+ }
Review Comment:
Can you add a UT for this?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on PR #5244: URL: https://github.com/apache/gravitino/pull/5244#issuecomment-2443442697 > I think you should think if user implement a custom FS with FS provider, can he integrate into Gravitino server Hadoop catalog, Java and Python gvfs without code change in Gravitino? Take this as a principle, if not, please make it work. In most cases, the current implementation can satisfy users' needs. They can use the `gravitino.bypass` mechanism to pass to keys with prefixes `gravitino.bypass` to the filesystem providers, the file system provider then will remove the prefix and get the real keys. or just directly pass the properties. For example, if we use the following key/value pair as the properties when creating a Hadoop catalog, ``` k1=value1 gravitino.bypass.k2=value2 ``` Then, we will pass these configurations on to the file system provider. It's quite flexible now and it depends on the users that custom a new file system provider what a property key stands for. key `k1` may stand for a core configuration or just a useless one. ``` k1=value1 k2=value2 ``` The following three keys are identical and users can use any key that contains endpoint information to the file system provider and then assign it to the key `fs.s3a.endpoint` ``` fs.s3a.endpoint = x s3-endpoint= gravitino.bypass.fs.s3a.endpoint = ``` I'm not very sure whether the kinds of property conversion are acceptable to you. If we need to specify each required property explicitly, like the current S3 implementation, users need to use `s3-endpoint` in the Gravitino server and `s3.endpoint` in the GVFS Java client to pass endpoint information, I believe we need to add a new interface method like `requireConfigurationKey()` to tell users what the properties they required to pass, I'm hesitant to do so as it seems to make thing more complex and not very extensive enough. -- 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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1819997848
##
catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopGCSCatalogIT.java:
##
@@ -95,10 +96,8 @@ protected String defaultBaseLocation() {
protected void createCatalog() {
Review Comment:
Yes, I can try it but I will not do it in this PR, It will make this PR
large and hard to test the changes introduced by this PR, so I created
https://github.com/apache/gravitino/issues/5325 to follow it.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820145095
##
bundles/aliyun-bundle/src/main/java/org/apache/gravitino/oss/fs/OSSFileSystemProvider.java:
##
@@ -18,23 +18,41 @@
*/
package org.apache.gravitino.oss.fs;
+import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Map;
import org.apache.gravitino.catalog.hadoop.fs.FileSystemProvider;
+import org.apache.gravitino.storage.OSSProperties;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.aliyun.oss.AliyunOSSFileSystem;
+import org.apache.hadoop.fs.aliyun.oss.Constants;
public class OSSFileSystemProvider implements FileSystemProvider {
+
+ private static final String OSS_FILESYSTEM_IMPL = "fs.oss.impl";
+
+ public static final Map GRAVITINO_KEY_TO_OSS_HADOOP_KEY =
+ ImmutableMap.of(
+ OSSProperties.GRAVITINO_OSS_ENDPOINT, Constants.ENDPOINT_KEY,
+ OSSProperties.GRAVITINO_OSS_ACCESS_KEY_ID, Constants.ACCESS_KEY_ID,
+ OSSProperties.GRAVITINO_OSS_ACCESS_KEY_SECRET,
Constants.ACCESS_KEY_SECRET);
+
@Override
public FileSystem getFileSystem(Path path, Map config)
throws IOException {
Configuration configuration = new Configuration();
config.forEach(
(k, v) -> {
- configuration.set(k.replace("gravitino.bypass.", ""), v);
+ if (k.startsWith(GRAVITINO_BYPASS)) {
+configuration.set(k.replace(GRAVITINO_BYPASS, ""), v);
+ } else
configuration.set(GRAVITINO_KEY_TO_OSS_HADOOP_KEY.getOrDefault(k, k), v);
Review Comment:
You'd better add `{ xxx }` for `else`.
##
bundles/aliyun-bundle/src/main/java/org/apache/gravitino/oss/fs/OSSFileSystemProvider.java:
##
@@ -18,23 +18,41 @@
*/
package org.apache.gravitino.oss.fs;
+import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Map;
import org.apache.gravitino.catalog.hadoop.fs.FileSystemProvider;
+import org.apache.gravitino.storage.OSSProperties;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.aliyun.oss.AliyunOSSFileSystem;
+import org.apache.hadoop.fs.aliyun.oss.Constants;
public class OSSFileSystemProvider implements FileSystemProvider {
+
+ private static final String OSS_FILESYSTEM_IMPL = "fs.oss.impl";
+
+ public static final Map GRAVITINO_KEY_TO_OSS_HADOOP_KEY =
Review Comment:
Why do we want to make this public?
##
bundles/gcp-bundle/src/main/java/org/apache/gravitino/gcs/fs/GCSFileSystemProvider.java:
##
@@ -31,12 +33,18 @@
public class GCSFileSystemProvider implements FileSystemProvider {
private static final Logger LOGGER =
LoggerFactory.getLogger(GCSFileSystemProvider.class);
+ public static final Map GRAVITINO_KEY_TO_GCS_HADOOP_KEY =
+ ImmutableMap.of(
+ GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH,
"fs.gs.auth.service.account.json.keyfile");
Review Comment:
Can you please define this "fs.gs.auth.service.account.json.keyfile" as
static variable.
##
bundles/gcp-bundle/build.gradle.kts:
##
@@ -34,6 +34,7 @@ dependencies {
implementation(libs.commons.lang3)
implementation(libs.hadoop3.gcs)
+ implementation(project(":catalogs:catalog-common"))
Review Comment:
Why don't we `exclude('*')` here?
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/fs/FileSystemProvider.java:
##
@@ -32,6 +32,8 @@
*/
public interface FileSystemProvider {
+ String GRAVITINO_BYPASS = "gravitino.bypass.";
Review Comment:
Can you please add some comments to this variable, to let users know how to
leverage this variable.
##
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/GravitinoVirtualFileSystemConfiguration.java:
##
@@ -107,5 +112,46 @@ public class GravitinoVirtualFileSystemConfiguration {
public static final long
FS_GRAVITINO_FILESET_CACHE_EVICTION_MILLS_AFTER_ACCESS_DEFAULT =
1000L * 60 * 60;
+ private static final Map GVFS_S3_KEY_TO_HADOOP_KEY =
Review Comment:
If we add a new FS provider, do we need to update the GVFS code to make it
work? If so, I think it is not a good design, we should also hide the client
side configuration into each provider.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1820046059
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogOperations.java:
##
@@ -771,6 +774,23 @@ FileSystem getFileSystem(Path path, Map
config) throws IOExcepti
scheme, path, fileSystemProvidersMap.keySet(),
fileSystemProvidersMap.values()));
}
-return provider.getFileSystem(path, config);
+return provider.getFileSystem(path, convertGravitinoToHadoopConf(config));
+ }
+
+ private Map convertGravitinoToHadoopConf(Map
gravitinoConf) {
+Map hadoopConf = Maps.newHashMap();
+
+gravitinoConf.forEach(
+(k, v) -> {
+ if
(HadoopS3FileSystemConfig.GRAVITINO_KEY_TO_S3_HADOOP_KEY.containsKey(k)) {
+
hadoopConf.put(HadoopS3FileSystemConfig.GRAVITINO_KEY_TO_S3_HADOOP_KEY.get(k),
v);
+ } else if
(HadoopGCSFileSystemConfig.GRAVITINO_KEY_TO_GCS_HADOOP_KEY.containsKey(k)) {
+
hadoopConf.put(HadoopGCSFileSystemConfig.GRAVITINO_KEY_TO_GCS_HADOOP_KEY.get(k),
v);
+ } else
+hadoopConf.put(
+
HadoopOSSFileSystemConfig.GRAVITINO_KEY_TO_OSS_HADOOP_KEY.getOrDefault(k, k),
v);
+});
+
+return hadoopConf;
Review Comment:
fixed
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1819997848
##
catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopGCSCatalogIT.java:
##
@@ -95,10 +96,8 @@ protected String defaultBaseLocation() {
protected void createCatalog() {
Review Comment:
Yes, I can try it but I will not do it in this PR, I will make this PR large
and hard to test the changes introduced by this PR, so I created
https://github.com/apache/gravitino/issues/5325 to follow it.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1819147093
##
catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopGCSCatalogIT.java:
##
@@ -95,10 +96,8 @@ protected String defaultBaseLocation() {
protected void createCatalog() {
Review Comment:
Can we move these ITs to each bundle?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1819151146
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/config/HadoopGCSFileSystemConfig.java:
##
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.catalog.hadoop.config;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+import org.apache.gravitino.connector.PropertyEntry;
+import org.apache.gravitino.storage.GCSProperties;
+
+public class HadoopGCSFileSystemConfig extends Config {
Review Comment:
I think you cannot guarantee all the configurations for each provider,
configurations belong to the provider, the implementor of the provider should
guarantee the validity of the configurations/properties, not Gravitino itself.
The implementation here breaks the plugin mechanism, and mix the frameworks and
implementation together.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1818890807
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogOperations.java:
##
@@ -771,6 +774,23 @@ FileSystem getFileSystem(Path path, Map
config) throws IOExcepti
scheme, path, fileSystemProvidersMap.keySet(),
fileSystemProvidersMap.values()));
}
-return provider.getFileSystem(path, config);
+return provider.getFileSystem(path, convertGravitinoToHadoopConf(config));
+ }
+
+ private Map convertGravitinoToHadoopConf(Map
gravitinoConf) {
+Map hadoopConf = Maps.newHashMap();
+
+gravitinoConf.forEach(
+(k, v) -> {
+ if
(HadoopS3FileSystemConfig.GRAVITINO_KEY_TO_S3_HADOOP_KEY.containsKey(k)) {
+
hadoopConf.put(HadoopS3FileSystemConfig.GRAVITINO_KEY_TO_S3_HADOOP_KEY.get(k),
v);
+ } else if
(HadoopGCSFileSystemConfig.GRAVITINO_KEY_TO_GCS_HADOOP_KEY.containsKey(k)) {
+
hadoopConf.put(HadoopGCSFileSystemConfig.GRAVITINO_KEY_TO_GCS_HADOOP_KEY.get(k),
v);
+ } else
+hadoopConf.put(
+
HadoopOSSFileSystemConfig.GRAVITINO_KEY_TO_OSS_HADOOP_KEY.getOrDefault(k, k),
v);
+});
+
+return hadoopConf;
Review Comment:
Yeah, I had hesitated about this method and finally decided to make the code
like the current one. I will modify it.
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/config/HadoopGCSFileSystemConfig.java:
##
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.catalog.hadoop.config;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+import org.apache.gravitino.connector.PropertyEntry;
+import org.apache.gravitino.storage.GCSProperties;
+
+public class HadoopGCSFileSystemConfig extends Config {
+
+ public static final Map GRAVITINO_KEY_TO_GCS_HADOOP_KEY =
+ ImmutableMap.of(
+ GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH,
"fs.gs.auth.service.account.json.keyfile");
+
+ public HadoopGCSFileSystemConfig(Map properties) {
+super(false);
+loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry HADOOP_OSS_ENDPOINT_ENTRY =
Review Comment:
It can be private, l will change it.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1818887843
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/config/HadoopGCSFileSystemConfig.java:
##
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.catalog.hadoop.config;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+import org.apache.gravitino.connector.PropertyEntry;
+import org.apache.gravitino.storage.GCSProperties;
+
+public class HadoopGCSFileSystemConfig extends Config {
Review Comment:
If we move them to the provider, we won't perform the valid check mechanism
provided by the Hadoop catalog. for example, if the user sets an empty value
for the s3 endpoint, Hadoop catalog can detect errors during the creation
process.
If we don't place much importance on this point, moving it to the provider
is acceptable to me.
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1816647366
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/config/HadoopGCSFileSystemConfig.java:
##
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.catalog.hadoop.config;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+import org.apache.gravitino.connector.PropertyEntry;
+import org.apache.gravitino.storage.GCSProperties;
+
+public class HadoopGCSFileSystemConfig extends Config {
+
+ public static final Map GRAVITINO_KEY_TO_GCS_HADOOP_KEY =
+ ImmutableMap.of(
+ GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH,
"fs.gs.auth.service.account.json.keyfile");
+
+ public HadoopGCSFileSystemConfig(Map properties) {
+super(false);
+loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry HADOOP_OSS_ENDPOINT_ENTRY =
+ new ConfigBuilder(GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH)
+ .doc("The path of service account json file of Google Cloud Storage")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public String getServiceAccountJsonPath() {
+return get(HADOOP_OSS_ENDPOINT_ENTRY);
+ }
+
+ public static final Map>
GCS_FILESYSTEM_PROPERTY_ENTRIES =
Review Comment:
do you mean we need to merge these tree configuration files into a single
one and it's preferable to combine them with other keys like `location`?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1816220170
##
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/GCSProperties.java:
##
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.storage;
+
+public class GCSProperties {
+
+ // The path of service account JSON file of Google Cloud Storage.
+ public static final String GCS_SERVICE_ACCOUNT_JSON_PATH =
"gcs-service-account-file";
+
+ public GCSProperties() {}
Review Comment:
Please change this to `private`.
##
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/S3Properties.java:
##
@@ -31,5 +31,8 @@ public class S3Properties {
// The region of the S3 service.
public static final String GRAVITINO_S3_REGION = "s3-region";
+ // The S3 credentials provider class name.
+ public static final String GRAVITINO_S3_CREDS_PROVIDER = "s3-creds-provider";
+
private S3Properties() {}
}
Review Comment:
> he key s3-creds-provider in Gravitino server and use s3.creds.providers
for GVFS(replace '-' with '.', other keys are similar), is that OK?
I'm OK with this.
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/config/HadoopGCSFileSystemConfig.java:
##
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.catalog.hadoop.config;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+import org.apache.gravitino.connector.PropertyEntry;
+import org.apache.gravitino.storage.GCSProperties;
+
+public class HadoopGCSFileSystemConfig extends Config {
+
+ public static final Map GRAVITINO_KEY_TO_GCS_HADOOP_KEY =
+ ImmutableMap.of(
+ GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH,
"fs.gs.auth.service.account.json.keyfile");
+
+ public HadoopGCSFileSystemConfig(Map properties) {
+super(false);
+loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry HADOOP_OSS_ENDPOINT_ENTRY =
+ new ConfigBuilder(GCSProperties.GCS_SERVICE_ACCOUNT_JSON_PATH)
+ .doc("The path of service account json file of Google Cloud Storage")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public String getServiceAccountJsonPath() {
+return get(HADOOP_OSS_ENDPOINT_ENTRY);
+ }
+
+ public static final Map>
GCS_FILESYSTEM_PROPERTY_ENTRIES =
Review Comment:
Also here.
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/HadoopCatalogOperations.java:
##
@@ -771,6 +774,23 @@ FileSystem getFileSystem(Path path, Map
config) throws IOExcepti
scheme, path, fileSystemProvidersMap.keySet(),
fileSystemProvidersMap.values()));
}
-return provider.getFileSystem(path, config);
+return provider.getFileSystem(path, convertGravitinoToHadoopConf(config));
+ }
+
+ private Map convertGravitinoToHadoopConf(Map
gravitinoConf) {
+Map hadoopConf = Maps.newHashMap();
+
+gravitinoConf.f
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1815956547
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/config/HadoopS3FileSystemConfig.java:
##
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.catalog.hadoop.config;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+import org.apache.gravitino.connector.PropertyEntry;
+import org.apache.gravitino.storage.S3Properties;
+
+public class HadoopS3FileSystemConfig extends Config {
+
+ public static final Map GRAVITINO_KEY_TO_S3_HADOOP_KEY =
+ ImmutableMap.of(
+ S3Properties.GRAVITINO_S3_ENDPOINT, "fs.s3a.endpoint",
+ S3Properties.GRAVITINO_S3_ACCESS_KEY_ID, "fs.s3a.access.key",
+ S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY, "fs.s3a.secret.key",
+ S3Properties.GRAVITINO_S3_CREDS_PROVIDER,
"fs.s3a.aws.credentials.provider");
+
+ public HadoopS3FileSystemConfig(Map properties) {
+super(false);
+loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry HADOOP_S3_ENDPOINT_ENTRY =
+ new ConfigBuilder(S3Properties.GRAVITINO_S3_ENDPOINT)
+ .doc("The endpoint of the AWS s3")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public static final ConfigEntry HADOOP_S3_ACCESS_KEY_ENTRY =
+ new ConfigBuilder(S3Properties.GRAVITINO_S3_ACCESS_KEY_ID)
+ .doc("The access key of the AWS s3")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public static final ConfigEntry HADOOP_S3_SECRET_KEY_ENTRY =
+ new ConfigBuilder(S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY)
+ .doc("The secret key of the AWS s3")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public static final ConfigEntry
HADOOP_S3_CREDENTIAL_PROVIDER_KEY_ENTRY =
+ new ConfigBuilder(S3Properties.GRAVITINO_S3_CREDS_PROVIDER)
+ .doc("The S3 credentials provider class name")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public static final Map>
S3_FILESYSTEM_PROPERTY_ENTRIES =
+ new ImmutableMap.Builder>()
+ .put(
+ S3Properties.GRAVITINO_S3_ENDPOINT,
+ PropertyEntry.stringOptionalPropertyEntry(
+ S3Properties.GRAVITINO_S3_ENDPOINT,
+ "The endpoint of the AWS s3",
+ false /* immutable */,
+ null /* defaultValue */,
+ false /* hidden */))
+ .put(
+ S3Properties.GRAVITINO_S3_ACCESS_KEY_ID,
+ PropertyEntry.stringOptionalPropertyEntry(
+ S3Properties.GRAVITINO_S3_ACCESS_KEY_ID,
+ "The access key of the AWS s3",
+ false /* immutable */,
+ null /* defaultValue */,
+ false /* hidden */))
+ .put(
+ S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY,
+ PropertyEntry.stringOptionalPropertyEntry(
+ S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY,
+ "The secret key of the AWS s3",
+ false /* immutable */,
+ null /* defaultValue */,
+ false /* hidden */))
+ .put(
+ S3Properties.GRAVITINO_S3_CREDS_PROVIDER,
Review Comment:
Why S3 has this credential provider property, but gcs doesn't?
--
This is an automated messag
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1815953943
##
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/S3Properties.java:
##
@@ -31,5 +31,8 @@ public class S3Properties {
// The region of the S3 service.
public static final String GRAVITINO_S3_REGION = "s3-region";
+ // The S3 credentials provider class name.
+ public static final String GRAVITINO_S3_CREDS_PROVIDER = "s3-creds-provider";
Review Comment:
Why we don't have OSS related properties defined?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1815957538
##
catalogs/catalog-hadoop/src/main/java/org/apache/gravitino/catalog/hadoop/config/HadoopS3FileSystemConfig.java:
##
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.catalog.hadoop.config;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.config.ConfigBuilder;
+import org.apache.gravitino.config.ConfigConstants;
+import org.apache.gravitino.config.ConfigEntry;
+import org.apache.gravitino.connector.PropertyEntry;
+import org.apache.gravitino.storage.S3Properties;
+
+public class HadoopS3FileSystemConfig extends Config {
+
+ public static final Map GRAVITINO_KEY_TO_S3_HADOOP_KEY =
+ ImmutableMap.of(
+ S3Properties.GRAVITINO_S3_ENDPOINT, "fs.s3a.endpoint",
+ S3Properties.GRAVITINO_S3_ACCESS_KEY_ID, "fs.s3a.access.key",
+ S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY, "fs.s3a.secret.key",
+ S3Properties.GRAVITINO_S3_CREDS_PROVIDER,
"fs.s3a.aws.credentials.provider");
+
+ public HadoopS3FileSystemConfig(Map properties) {
+super(false);
+loadFromMap(properties, k -> true);
+ }
+
+ public static final ConfigEntry HADOOP_S3_ENDPOINT_ENTRY =
+ new ConfigBuilder(S3Properties.GRAVITINO_S3_ENDPOINT)
+ .doc("The endpoint of the AWS s3")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public static final ConfigEntry HADOOP_S3_ACCESS_KEY_ENTRY =
+ new ConfigBuilder(S3Properties.GRAVITINO_S3_ACCESS_KEY_ID)
+ .doc("The access key of the AWS s3")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public static final ConfigEntry HADOOP_S3_SECRET_KEY_ENTRY =
+ new ConfigBuilder(S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY)
+ .doc("The secret key of the AWS s3")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public static final ConfigEntry
HADOOP_S3_CREDENTIAL_PROVIDER_KEY_ENTRY =
+ new ConfigBuilder(S3Properties.GRAVITINO_S3_CREDS_PROVIDER)
+ .doc("The S3 credentials provider class name")
+ .version(ConfigConstants.VERSION_0_7_0)
+ .stringConf()
+ .checkValue(StringUtils::isNotBlank,
ConfigConstants.NOT_BLANK_ERROR_MSG)
+ .create();
+
+ public static final Map>
S3_FILESYSTEM_PROPERTY_ENTRIES =
+ new ImmutableMap.Builder>()
+ .put(
+ S3Properties.GRAVITINO_S3_ENDPOINT,
+ PropertyEntry.stringOptionalPropertyEntry(
+ S3Properties.GRAVITINO_S3_ENDPOINT,
+ "The endpoint of the AWS s3",
+ false /* immutable */,
+ null /* defaultValue */,
+ false /* hidden */))
+ .put(
+ S3Properties.GRAVITINO_S3_ACCESS_KEY_ID,
+ PropertyEntry.stringOptionalPropertyEntry(
+ S3Properties.GRAVITINO_S3_ACCESS_KEY_ID,
+ "The access key of the AWS s3",
+ false /* immutable */,
+ null /* defaultValue */,
+ false /* hidden */))
+ .put(
+ S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY,
+ PropertyEntry.stringOptionalPropertyEntry(
+ S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY,
+ "The secret key of the AWS s3",
+ false /* immutable */,
+ null /* defaultValue */,
+ false /* hidden */))
+ .put(
+ S3Properties.GRAVITINO_S3_CREDS_PROVIDER,
Review Comment:
Yes, Gcs only needs to specify the service JSON file.
--
This is an automated message from th
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1815957219
##
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/S3Properties.java:
##
@@ -31,5 +31,8 @@ public class S3Properties {
// The region of the S3 service.
public static final String GRAVITINO_S3_REGION = "s3-region";
+ // The S3 credentials provider class name.
+ public static final String GRAVITINO_S3_CREDS_PROVIDER = "s3-creds-provider";
+
private S3Properties() {}
}
Review Comment:
The default value of this key is `EC2ContainerCredentialsProviderWrapper`
which is NOT `BasicAWSCredentialsProvider` for S3, however, the corresponding
value for `OSS` is the one that uses AS/SK, so we do not need to add this one.
Another problem, we use
the key `s3-creds-provider` in Gravitino server and use `s3.creds.providers`
for GVFS(replace '-' with '.', other keys are similar), is that OK?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1815957219
##
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/S3Properties.java:
##
@@ -31,5 +31,8 @@ public class S3Properties {
// The region of the S3 service.
public static final String GRAVITINO_S3_REGION = "s3-region";
+ // The S3 credentials provider class name.
+ public static final String GRAVITINO_S3_CREDS_PROVIDER = "s3-creds-provider";
+
private S3Properties() {}
}
Review Comment:
The default value of this key is `EC2ContainerCredentialsProviderWrapper`
which is NOT `BasicAWSCredentialsProvider` for S3, however, the corresponding
value for `OSS` is the one that uses AS/SK, so we do not need to add this one.
Another problem, we use
the key `s3-creds-provider` in Gravitino server and use `s3.creds.providers`
for GVFS, is that OK?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
yuqi1129 commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1815957219
##
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/S3Properties.java:
##
@@ -31,5 +31,8 @@ public class S3Properties {
// The region of the S3 service.
public static final String GRAVITINO_S3_REGION = "s3-region";
+ // The S3 credentials provider class name.
+ public static final String GRAVITINO_S3_CREDS_PROVIDER = "s3-creds-provider";
+
private S3Properties() {}
}
Review Comment:
The default value of this key is `EC2ContainerCredentialsProviderWrapper`
which is NOT `BasicAWSCredentialsProvider`, however, the corresponding value
for `OSS` is the one that uses AS/SK, so we do not need to add this one.
Another problem, we use
the key `s3-creds-provider` in Gravitino server and use `s3.creds.providers`
for GVFS, is that OK?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1815954677
##
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/S3Properties.java:
##
@@ -31,5 +31,8 @@ public class S3Properties {
// The region of the S3 service.
public static final String GRAVITINO_S3_REGION = "s3-region";
+ // The S3 credentials provider class name.
+ public static final String GRAVITINO_S3_CREDS_PROVIDER = "s3-creds-provider";
+
private S3Properties() {}
}
Review Comment:
Why we don't have OSS related properties defined here in catalog-common?
--
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]
Re: [PR] [#5220] improvment(hadoop-catalog): Optimize the name properties keys for Hadoop catalog. [gravitino]
jerryshao commented on code in PR #5244:
URL: https://github.com/apache/gravitino/pull/5244#discussion_r1815953943
##
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/S3Properties.java:
##
@@ -31,5 +31,8 @@ public class S3Properties {
// The region of the S3 service.
public static final String GRAVITINO_S3_REGION = "s3-region";
+ // The S3 credentials provider class name.
+ public static final String GRAVITINO_S3_CREDS_PROVIDER = "s3-creds-provider";
Review Comment:
Why we don't have OSS related properties defined?
--
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]
