Skip to content

Core: Re-add and deprecate HMS_TABLE_OWNER to TableProperties#6314

Merged
RussellSpitzer merged 1 commit into
apache:masterfrom
nastra:hms-table-owner-property
Nov 30, 2022
Merged

Core: Re-add and deprecate HMS_TABLE_OWNER to TableProperties#6314
RussellSpitzer merged 1 commit into
apache:masterfrom
nastra:hms-table-owner-property

Conversation

@nastra
Copy link
Copy Markdown
Contributor

@nastra nastra commented Nov 30, 2022

This table property was shipped with 1.1.0 and then removed by #6045 before we configured RevAPI to compare against 1.1.0 in #6275

CI is currently failing on master with:

    * Just this break:
        ./gradlew :iceberg-core:revapiAcceptBreak --justification "{why this break is ok}" \
          --code "java.field.removedWithConstant" \
          --old "field org.apache.iceberg.TableProperties.HMS_TABLE_OWNER"
    * All breaks in this project:
        ./gradlew :iceberg-core:revapiAcceptAllBreaks --justification "{why this break is ok}"
    * All breaks in all projects:
        ./gradlew revapiAcceptAllBreaks --justification "{why this break is ok}"
  ----------------------------------------------------------------------------------------------------

/cc @szehon-ho @haizhou-zhao

@gaborkaszab
Copy link
Copy Markdown
Contributor

LGTM, makes sense!

I tried to create a milestone for 1.3 to include deprecating TableProperties.HMS_TABLE_OWNER so that we won't forget, but I haven't found a way to create that milestone (I guess I lack permissions). I created the issue though, to drop this field. @Fokko could you please take a look if you can create that milestone?

@Fokko Fokko added this to the Iceberg 1.3.0 milestone Nov 30, 2022
@Fokko
Copy link
Copy Markdown
Contributor

Fokko commented Nov 30, 2022

@gaborkaszab Sure, created and added 👍🏻

@gaborkaszab gaborkaszab removed this from the Iceberg 1.3.0 milestone Nov 30, 2022
@gaborkaszab
Copy link
Copy Markdown
Contributor

gaborkaszab commented Nov 30, 2022

@Fokko Thanks for creating 1.3.0 milestone! However, I see you added this ticket to the milestone but this in fact is meant to be in 1.2.0 to deprecate that field, and #6316 is meant to be in 1.3.0 to drop it. I added it accordingly and removed this one.

This PR requires immediate attention I guess as it breaks the build now.

Copy link
Copy Markdown
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think restoring this is the easiest way to fix this. I through this was safe to delete, but it was part of the 1.1.0 release.

@RussellSpitzer RussellSpitzer merged commit ae15c7e into apache:master Nov 30, 2022
Copy link
Copy Markdown
Member

@RussellSpitzer RussellSpitzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@nastra nastra deleted the hms-table-owner-property branch November 30, 2022 15:49
@RussellSpitzer
Copy link
Copy Markdown
Member

Thanks @nastra for getting this! Thanks @Fokko for the review

@szehon-ho
Copy link
Copy Markdown
Member

Thanks a lot for the fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants