stevenzwu commented on code in PR #12315:
URL: https://github.com/apache/iceberg/pull/12315#discussion_r1962236768


##########
core/src/main/java/org/apache/iceberg/LocationProviders.java:
##########
@@ -75,6 +77,23 @@ public static LocationProvider locationsFor(
     }
   }
 
+  private static final Set<String> DEPRECATED_PROPERTIES =
+      ImmutableSet.of(
+          TableProperties.OBJECT_STORE_PATH, 
TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
+
+  private static String getProperty(Map<String, String> properties, String 
key) {

Review Comment:
   `checkDeprecation` is also not accurate. This method actually does both: get 
property value and check deprecations.
   
    the implementation/error msg is tied to 
`TableProperties.WRITE_DATA_LOCATION`. Maybe call this `getLegacyDataLocation` 
or sth like that.
   
   The other approach is to define `DEPRECATED_PROPERTIES` in `TableProperties` 
class as a map (key = deprecated property, value = new property). maybe we can 
get to it when the need comes.  right now, we can keep the change local to 
`LocationProviders` class



-- 
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: issues-unsubscr...@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to