Copilot commented on code in PR #55675:
URL: https://github.com/apache/doris/pull/55675#discussion_r2326371414
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java:
##########
@@ -127,14 +129,36 @@ public Map<StorageProperties.Type, StorageProperties>
getStoragePropertiesMap()
.collect(Collectors.toMap(StorageProperties::getType, Function.identity()));
} catch (UserException e) {
LOG.warn("Failed to initialize catalog storage
properties", e);
- throw new RuntimeException("Failed to initialize
storage properties for catalog", e);
+ throw new RuntimeException("Failed to initialize
storage properties, error: "
+ + ExceptionUtils.getRootCauseMessage(e), e);
}
}
}
}
return storagePropertiesMap;
}
+ public void checkMetaStoreAndStorageProperties(Class msClass) {
+ MetastoreProperties msProperties;
+ List<StorageProperties> storageProperties;
+ try {
+ msProperties = MetastoreProperties.create(getProperties());
+ storageProperties = StorageProperties.createAll(getProperties());
+ } catch (UserException e) {
+ throw new RuntimeException("Failed to initialize Catalog
properties, error: "
+ + ExceptionUtils.getRootCauseMessage(e), e);
+ }
+ Preconditions.checkNotNull(storageProperties,
+ "Storage properties are not configured properly");
+ Preconditions.checkNotNull(msProperties, "Metastore properties are not
configured properly");
+ Preconditions.checkArgument(
+ msClass.isInstance(msProperties),
+ "Metastore properties type is not correct. Expected %s but got
%s"
+ + msClass.getName()
+ + msProperties.getClass().getName());
Review Comment:
The error message string formatting is incorrect. It contains '%s'
placeholders but no corresponding arguments are provided. Either use
String.format() with proper arguments or remove the placeholders.
```suggestion
String.format("Metastore properties type is not correct.
Expected %s but got %s",
msClass.getName(),
msProperties.getClass().getName()));
```
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java:
##########
@@ -150,7 +174,8 @@ public MetastoreProperties getMetastoreProperties() {
metastoreProperties =
MetastoreProperties.create(getProperties());
} catch (UserException e) {
LOG.warn("Failed to create metastore properties", e);
- throw new RuntimeException("Failed to create metastore
properties", e);
+ throw new RuntimeException("Failed to create metastore
properties, error: "
+ + ExceptionUtils.getRootCause(e), e);
Review Comment:
Inconsistent exception handling - line 133 uses
`ExceptionUtils.getRootCauseMessage(e)` while line 178 uses
`ExceptionUtils.getRootCause(e)`. Use `getRootCauseMessage()` consistently for
string concatenation.
```suggestion
+ ExceptionUtils.getRootCauseMessage(e), e);
```
##########
fe/fe-core/src/test/java/org/apache/doris/common/util/LocationPathTest.java:
##########
@@ -39,6 +39,14 @@ public class LocationPathTest {
static {
Map<String, String> props = new HashMap<>();
props.put("dfs.nameservices", "namenode:8020");
Review Comment:
Duplicate key 'dfs.nameservices' - the second assignment on line 42
overwrites the first assignment on line 41. Remove line 41.
```suggestion
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
