> When you migrate a table, I don't think everyone cleans up the old properties
That is exactly the scenario we are trying to guard against. Maybe old stables are still using those deprecated properties. Hence we want to guard against silent behavior change. > and then jobs start failing. Warning logs can be too gentle and missed by users. It is like the deprecation warning in code API. Failing the table loading is like the compiling error for removed code API. This is the explicit signal. The error msg can provide explicit instruction on how to fix it (maybe with a link to doc/announcement). If we want a real gentle deprecation process for table properties, I guess the full steps can be - 1.9 - warning logs with explicit msg that table loading shall fail in a future release (like 1.10) - 1.10 - fail the table loading - 2.0 - remove all references to deprecated table properties On Tue, Feb 18, 2025 at 9:57 AM Fokko Driesprong <fo...@apache.org> wrote: > I'm hesitant to fail the job. When you migrate a table, I don't think > everyone cleans up the old properties, and then jobs start failing. > > Another approach is to warn until 2.0, and then remove them: > https://github.com/apache/iceberg/pull/12315 > > LMKWYT > > Kind regards, > Fokko > > Op di 18 feb 2025 om 16:25 schreef Robert Stupp <sn...@snazy.de>: > >> Also, as an idea, REST catalog services could return an error if those >> deprecated properties are being set. Thoughts? >> On 18.02.25 16:21, Robert Stupp wrote: >> >> Agree with both Steve's. Personally, I'm okay with removing those >> properties - but using the proposed phased approach. >> On 17.02.25 23:25, Steven Wu wrote: >> >> I have some concerns on the issue of silent behavior change that Steve >> Zhang raised in the PR comment. E.g., users may set the location based on >> the deprecated table property, With this change, it would silently switch >> to a new location. This can potentially mess up orphan file cleanup etc. >> >> Maybe we should consider the more conservative two-step approach that >> Steve mentioned (1) in the next release of 1.9, fail when those properties >> were defined in the table (2) remove those properties' references maybe in >> 2.0. >> >> On Mon, Feb 17, 2025 at 2:17 PM Kevin Liu <kevinjq...@apache.org> wrote: >> >>> +1 for removing. Thanks for taking up the cleanup duty! >>> >>> I looked up the usage for the property and its string value with github >>> search, and confirmed that they are not used. >>> >>> Also, for reference, here are the previous related PRs: >>> https://github.com/apache/iceberg/pull/3094 >>> https://github.com/apache/iceberg/pull/2965 >>> >>> Best, >>> Kevin Liu >>> >>> On Mon, Feb 17, 2025 at 2:06 PM Yufei Gu <flyrain...@gmail.com> wrote: >>> >>>> +1 to remove them. >>>> Yufei >>>> >>>> >>>> On Mon, Feb 17, 2025 at 1:26 PM Steve Zhang >>>> <hongyue_zh...@apple.com.invalid> <hongyue_zh...@apple.com.invalid> >>>> wrote: >>>> >>>>> Thanks Fokko for removing deprecated properties! >>>>> >>>>> Just want to highlight the worst case for tables with old >>>>> configuration and not aware of this deprecation might experience silent >>>>> behavior change. But considering this has been deprecated for past 3 >>>>> years, >>>>> here’s my +1. >>>>> >>>>> Thanks, >>>>> Steve Zhang >>>>> >>>>> >>>>> >>>>> On Feb 17, 2025, at 2:18 AM, Fokko Driesprong <fo...@apache.org> >>>>> wrote: >>>>> >>>>> Hi everyone, >>>>> >>>>> While reviewing the LocationProvider equivalent of PyIceberg, I >>>>> noticed some old code in the Java codebase that I felt could be cleaned >>>>> up. You >>>>> can find the PR over here >>>>> <https://github.com/apache/iceberg/pull/12174>. This one removes the >>>>> deprecated properties: >>>>> >>>>> OBJECT_STORE_PATH = "write.object-storage.path"; >>>>> WRITE_FOLDER_STORAGE_LOCATION = "write.folder-storage.path"; >>>>> >>>>> These have been deprecated since Iceberg 0.12, and would like to know >>>>> if anyone has any concerns about removing these. >>>>> >>>>> Kind regards, >>>>> Fokko >>>>> >>>>> >>>>> -- >> Robert Stupp >> @snazy >> >> -- >> Robert Stupp >> @snazy >> >>
