raunaqmorarka commented on code in PR #18188:
URL: https://github.com/apache/iceberg/pull/18188#discussion_r4061869926


##########
core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java:
##########
@@ -162,16 +165,41 @@ public TableMetadata current() {
     return current;
   }
 
+  /** Seeds the ETag of the load response that produced {@link #current()}. */
+  void eTag(String newETag) {

Review Comment:
   Done. The ETag is now passed through the constructor; added a `newTableOps` 
overload taking `eTag` and deprecated the old one, which delegates with `null`.
   



##########
core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java:
##########
@@ -162,16 +165,41 @@ public TableMetadata current() {
     return current;
   }
 
+  /** Seeds the ETag of the load response that produced {@link #current()}. */
+  void eTag(String newETag) {
+    this.eTag = newETag;
+  }
+
   @Override
   public TableMetadata refresh() {
     Endpoint.check(endpoints, Endpoint.V1_LOAD_TABLE);
-    return updateCurrentMetadata(
+    Map<String, String> responseHeaders = 
Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER);
+    LoadTableResponse response =
         client.get(
             path,
             readQueryParams,
             LoadTableResponse.class,
-            readHeaders,
-            ErrorHandlers.tableErrorHandler()));
+            refreshHeaders(),
+            ErrorHandlers.tableErrorHandler(),
+            responseHeaders::putAll);
+
+    if (response == null) {
+      // 304 Not Modified: the server confirmed the metadata behind the ETag 
is still current
+      return current;
+    }
+
+    return updateCurrentMetadata(response, responseHeaders);
+  }
+
+  private Map<String, String> refreshHeaders() {

Review Comment:
   Renamed to `readHeadersWithETag`.
   



##########
core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java:
##########
@@ -162,16 +165,41 @@ public TableMetadata current() {
     return current;
   }
 
+  /** Seeds the ETag of the load response that produced {@link #current()}. */
+  void eTag(String newETag) {
+    this.eTag = newETag;
+  }
+
   @Override
   public TableMetadata refresh() {
     Endpoint.check(endpoints, Endpoint.V1_LOAD_TABLE);
-    return updateCurrentMetadata(
+    Map<String, String> responseHeaders = 
Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER);
+    LoadTableResponse response =
         client.get(
             path,
             readQueryParams,
             LoadTableResponse.class,
-            readHeaders,
-            ErrorHandlers.tableErrorHandler()));
+            refreshHeaders(),
+            ErrorHandlers.tableErrorHandler(),
+            responseHeaders::putAll);
+
+    if (response == null) {
+      // 304 Not Modified: the server confirmed the metadata behind the ETag 
is still current

Review Comment:
   Done.
   



##########
core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java:
##########
@@ -306,7 +342,8 @@ private static Long 
expectedSnapshotIdIfSnapshotAddOnly(List<MetadataUpdate> upd
     return addedSnapshotId;
   }
 
-  private TableMetadata updateCurrentMetadata(LoadTableResponse response) {
+  private TableMetadata updateCurrentMetadata(

Review Comment:
   Done.
   



##########
core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java:
##########
@@ -315,6 +352,9 @@ private TableMetadata 
updateCurrentMetadata(LoadTableResponse response) {
       this.current = checkUUID(current, response.tableMetadata());
     }
 
+    // a response without an ETag disables conditional refresh until the 
server sends one again

Review Comment:
   Removed.
   



-- 
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]

Reply via email to