ferenc-csaky commented on code in PR #23063:
URL: https://github.com/apache/flink/pull/23063#discussion_r1285576134
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/FileCatalogStore.java:
##########
@@ -97,25 +114,29 @@ public void storeCatalog(String catalogName,
CatalogDescriptor catalog)
throws CatalogException {
checkOpenState();
- Path filePath = getCatalogPath(catalogName);
+ Path catalogPath = getCatalogPath(catalogName);
try {
- File file = filePath.toFile();
- if (file.exists()) {
+ FileSystem fs = catalogPath.getFileSystem();
+
+ if (fs.exists(catalogPath)) {
throw new CatalogException(
String.format(
"Catalog %s's store file %s is already exist.",
- catalogName, filePath));
+ catalogName, catalogPath));
}
- // create a new file
- file.createNewFile();
- String yamlString =
yaml.dumpAsMap(catalog.getConfiguration().toMap());
- FileUtils.writeFile(file, yamlString, charset);
- LOG.info("Catalog {}'s configuration saved to file {}",
catalogName, filePath);
- } catch (Throwable e) {
+
+ try (FSDataOutputStream os = fs.create(catalogPath,
WriteMode.NO_OVERWRITE)) {
+ YAML_MAPPER.writeValue(os, catalog.getConfiguration().toMap());
+ }
+
+ LOG.info("Catalog {}'s configuration saved to file {}",
catalogName, catalogPath);
+ } catch (CatalogException e) {
+ throw e;
Review Comment:
The `Exception` catch clause will catch and rewrap `CatalogException` as
well, which unnecessary creates another exception stack element for any
expected `CatalogException`.
For example:
```java
try {
throw new DummyException("test");
} catch (Exception e) {
throw new DummyException(e);
}
```
Will produce 2 element exception stack trace:
```
Exception in thread "main" com.sandbox.Dummy$DummyException:
com.sandbox.Dummy$DummyException: test
at com.sandbox.Dummy.main(Dummy.java:8)
Caused by: com.sandbox.Dummy$DummyException: test
at com.sandbox.Dummy.main(Dummy.java:6)
```
While:
```java
try {
throw new DummyException("test");
} catch (DummyException e) {
throw e;
} catch (Exception e) {
throw new DummyException(e);
}
```
Will produce:
```
Exception in thread "main" com.sandbox.Dummy$DummyException: test
at com.sandbox.Dummy.main(Dummy.java:6)
```
The only reason of the separate catch clause is to avoid the rewrapping of
the expected exception.
--
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]
