dramaticlly commented on code in PR #12174:
URL: https://github.com/apache/iceberg/pull/12174#discussion_r1945589569
##########
core/src/main/java/org/apache/iceberg/LocationProviders.java:
##########
@@ -137,13 +134,7 @@ static class ObjectStoreLocationProvider implements
LocationProvider {
private static String dataLocation(Map<String, String> properties, String
tableLocation) {
String dataLocation =
properties.get(TableProperties.WRITE_DATA_LOCATION);
if (dataLocation == null) {
- dataLocation = properties.get(TableProperties.OBJECT_STORE_PATH);
- if (dataLocation == null) {
- dataLocation =
properties.get(TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
- if (dataLocation == null) {
- dataLocation = String.format("%s/data", tableLocation);
- }
- }
+ dataLocation = String.format("%s/data", tableLocation);
Review Comment:
I enjoy remove deprecated method or config but it's always a bit risky.
From a iceberg user perspective, if I had a running iceberg data pipeline, I
might not ever going to change/move away from deprecated one.
So in a worst case scenario for a given table with `OBJECT_STORE_PATH` or
`WRITE_FOLDER_STORAGE_LOCATION` is set and its value is not equal to
`WRITE_DATA_LOCATION`, shall we abort the write to force user action?
I think current logic is ignore the old table properties and directly write
into default, I think it might be a reasonable but want to know this is our
expectation. Maybe we can raise the awareness on dev list about the change?
##########
core/src/main/java/org/apache/iceberg/LocationProviders.java:
##########
@@ -137,13 +134,7 @@ static class ObjectStoreLocationProvider implements
LocationProvider {
private static String dataLocation(Map<String, String> properties, String
tableLocation) {
String dataLocation =
properties.get(TableProperties.WRITE_DATA_LOCATION);
if (dataLocation == null) {
- dataLocation = properties.get(TableProperties.OBJECT_STORE_PATH);
- if (dataLocation == null) {
- dataLocation =
properties.get(TableProperties.WRITE_FOLDER_STORAGE_LOCATION);
- if (dataLocation == null) {
- dataLocation = String.format("%s/data", tableLocation);
- }
- }
+ dataLocation = String.format("%s/data", tableLocation);
Review Comment:
I enjoy remove deprecated method or config but it's always a bit risky.
From a iceberg user perspective, if I had a running iceberg data pipeline, I
might not ever going to change/move away from deprecated one.
So in a worst case scenario for a given table with `OBJECT_STORE_PATH` or
`WRITE_FOLDER_STORAGE_LOCATION` is set and its value is not equal to
`WRITE_DATA_LOCATION`, shall we abort the write to force user action?
I think current logic is ignoring the old table properties and directly
write into default, I think it might be a reasonable but want to know this is
our expectation. Maybe we can raise the awareness on dev list about the change?
--
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
