Copilot commented on code in PR #9221:
URL: https://github.com/apache/gravitino/pull/9221#discussion_r2556388330
##########
flink-connector/flink/src/main/java/org/apache/gravitino/flink/connector/store/GravitinoCatalogStoreFactory.java:
##########
@@ -56,12 +56,15 @@ public void open(Context context) throws CatalogException {
factoryHelper.validate();
ReadableConfig options = factoryHelper.getOptions();
- String gravitinoUri =
- Preconditions.checkNotNull(
- options.get(GRAVITINO_URI), "The %s must be set.",
GRAVITINO_URI.key());
- String gravitinoName =
- Preconditions.checkNotNull(
- options.get(GRAVITINO_METALAKE), "The %s must be set.",
GRAVITINO_METALAKE.key());
+
+ String gravitinoUri = options.get(GRAVITINO_URI);
+ String gravitinoName = options.get(GRAVITINO_METALAKE);
+ Preconditions.checkArgument(
+ gravitinoUri != null && gravitinoName != null,
+ "Both %s and %s must be set",
+ GRAVITINO_METALAKE.key(),
+ GRAVITINO_URI.key());
Review Comment:
The error message arguments are in the wrong order. The message format says
"Both %s and %s must be set" but the arguments are provided as
`GRAVITINO_METALAKE.key(), GRAVITINO_URI.key()` when the variables checked are
in the order `gravitinoUri` and `gravitinoName`. The message should either be
"Both %s and %s must be set" with args `GRAVITINO_URI.key(),
GRAVITINO_METALAKE.key()` (to match the check order) or the message should be
reworded to match the current argument order.
```suggestion
GRAVITINO_URI.key(),
GRAVITINO_METALAKE.key());
```
##########
flink-connector/flink/src/test/java/org/apache/gravitino/flink/connector/integration/test/paimon/FlinkPaimonCatalogIT.java:
##########
@@ -74,7 +74,7 @@ void paimonSetup() {
@AfterAll
void paimonStop() {
- Preconditions.checkNotNull(metalake);
+ Preconditions.checkArgument(metalake != null);
Review Comment:
Missing error message in the checkArgument call. All other similar
checkArgument calls in this PR include an error message describing what should
not be null. This should have a message like "metalake should not be null" to
be consistent.
```suggestion
Preconditions.checkArgument(metalake != null, "metalake should not be
null");
```
--
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]
