the-other-tim-brown commented on code in PR #569:
URL: https://github.com/apache/incubator-xtable/pull/569#discussion_r1865045181


##########
xtable-core/src/main/java/org/apache/xtable/delta/DeltaConversionTarget.java:
##########
@@ -215,13 +224,64 @@ public String getTableFormat() {
     return TableFormat.DELTA;
   }
 
+  @Override
+  public Optional<String> getTargetCommitIdentifier(String sourceIdentifier) {

Review Comment:
   If there is no commit Identifier in the latest commit, should this just 
return an empty Optional?



##########
xtable-api/src/main/java/org/apache/xtable/spi/sync/ConversionTarget.java:
##########
@@ -76,8 +76,9 @@ public interface ConversionTarget {
    * Starts the sync and performs any initialization required
    *
    * @param table the table that will be synced
+   * @param sourceIdentifier
    */
-  void beginSync(InternalTable table);
+  void beginSync(InternalTable table, String sourceIdentifier);

Review Comment:
   I am wondering if it makes sense to attach the source format as part of the 
identifier. Then we can also have some metadata about what the writer format 
was. 



-- 
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-unsubscr...@xtable.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to