Fokko commented on code in PR #12315:
URL: https://github.com/apache/iceberg/pull/12315#discussion_r1961943022
##########
core/src/main/java/org/apache/iceberg/LocationProviders.java:
##########
@@ -88,6 +88,12 @@ private static String dataLocation(Map<String, String>
properties, String tableL
dataLocation =
properties.get(TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
if (dataLocation == null) {
dataLocation = String.format("%s/data", tableLocation);
+ } else {
+ throw new UnsupportedOperationException(
+ String.format(
+ "Property '%s' has been deprecated and will be removed in
2.0, use '%s' instead.",
+ TableProperties.WRITE_FOLDER_STORAGE_LOCATION,
+ TableProperties.WRITE_DATA_LOCATION));
Review Comment:
I've refactored the code to use the `DEPRECATED_PROPERTIES`.
My opinion is that we should not fail too fast, because that's annoying to
the user. For example, here when you have `write.data.path` and any deprecated
property, it will not fail because `write.data.path` takes priority. Also,
other properties, [such as
`write.manifest-lists.enabled`](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableProperties.java#L278-L286)
are harmless to have them, since it is now enabled by default. Fail on reading
or writing any deprecated property is a bigger discussion, and should happen on
the spec level because this is bigger than just the Java implementation, and
would like to decouple that from 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: 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
