You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@xtable.apache.org by "the-other-tim-brown (via GitHub)" <gi...@apache.org> on 2026/07/06 01:56:16 UTC
Re: [PR] feat: cli to fail on sync error [incubator-xtable]
the-other-tim-brown commented on code in PR #794:
URL: https://github.com/apache/incubator-xtable/pull/794#discussion_r3526076200
##########
xtable-utilities/src/main/java/org/apache/xtable/utilities/RunSync.java:
##########
@@ -183,9 +192,16 @@ static void syncTableMetdata(
.syncMode(SyncMode.INCREMENTAL)
.build();
try {
- conversionController.sync(conversionConfig, conversionSourceProvider);
+ Map<String, SyncResult> syncResults =
+ conversionController.sync(conversionConfig, conversionSourceProvider);
+ if (failOnError && hasSyncFailures(syncResults)) {
+ throw new RuntimeException("Sync completed with failures. See logs for details.");
+ }
} catch (Exception e) {
log.error("Error running sync for {}", table.getTableBasePath(), e);
+ if (failOnError) {
+ throw e;
Review Comment:
It looks like the caller is also catching an exception so lets take this opportunity to clean this up and only catch once.
##########
xtable-utilities/src/main/java/org/apache/xtable/utilities/RunCatalogSync.java:
##########
@@ -190,15 +200,38 @@ public static void main(String[] args) throws Exception {
.distinct()
.collect(Collectors.toList());
try {
- conversionController.syncTableAcrossCatalogs(
- conversionConfig,
- getConversionSourceProviders(tableFormats, tableFormatConverters, hadoopConf));
+ Map<String, SyncResult> syncResults =
+ conversionController.syncTableAcrossCatalogs(
+ conversionConfig,
+ getConversionSourceProviders(tableFormats, tableFormatConverters, hadoopConf));
+ if (failOnError && hasSyncFailures(syncResults)) {
+ throw new RuntimeException("Sync completed with failures. See logs for details.");
+ }
} catch (Exception e) {
log.error("Error running sync for {}", sourceTable.getBasePath(), e);
+ if (failOnError) {
+ throw e;
+ }
}
}
}
+ private static boolean hasSyncFailures(Map<String, SyncResult> syncResults) {
+ return syncResults.values().stream().anyMatch(RunCatalogSync::syncResultHasFailure);
+ }
+
+ private static boolean syncResultHasFailure(SyncResult syncResult) {
Review Comment:
Can we make this an instance method on the `SyncResult` instead? Then we can avoid duplicating the logic between the two files in this PR.
--
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: commits-unsubscribe@xtable.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org