Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove table module (2nd attempt) #17762

Merged
merged 2 commits into from
Jul 12, 2023
Merged

Conversation

apc999
Copy link
Contributor

@apc999 apc999 commented Jul 12, 2023

What changes are proposed in this pull request?

This commit represents a second effort at removing the table module along with its associated definitions and properties. Our initial attempt, PR #17220, was reverted due to a conflict with Presto and Trino codebases, as some removed classes were still being referenced.

Unlike the previous attempt, this commit ensures that all externally referenced classes (e.g., TableMasterClient.java) remain within the codebase to prevent blocking older Presto/Trino builds with newer Alluxio clients post table module removal.

Why are the changes needed?

Structured data service gets depreciated in 2.9 and should be removed in 300 or later.

Does this PR introduce any user facing changes?

Please list the user-facing changes introduced by your change, including

  1. catalog service removed
  2. transformation removed
  3. table command removed (CLI already removed in 2.9)

This commit fully remove the Table module and related properities and proto

This reverts commit c8d6991.
@apc999 apc999 requested review from JiamingMai and beinan July 12, 2023 05:26
@apc999
Copy link
Contributor Author

apc999 commented Jul 12, 2023

@JiamingMai @beinan, the first commit simply removes the table module by reverting the revert PR made by @JiamingMai. The second commit is the critical one that enables building with older versions of Trino/Presto. It does so by maintaining a select few necessary files.

Copy link
Contributor

@beinan beinan left a comment

Choose a reason for hiding this comment

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

lgtm, many thanks for cleaning up our code!

@apc999
Copy link
Contributor Author

apc999 commented Jul 12, 2023

alluxio-bot, merge this please

@alluxio-bot
Copy link
Contributor

merge failed:
Merge refused because pull request does not have label start with type-

@apc999 apc999 added area-structured-data Use this for items related to the structured data API and data transformations type-debt This issue is about tech debt type-code-quality code quality improvement labels Jul 12, 2023
@apc999
Copy link
Contributor Author

apc999 commented Jul 12, 2023

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 2b8b177 into Alluxio:main Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-structured-data Use this for items related to the structured data API and data transformations type-code-quality code quality improvement type-debt This issue is about tech debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants