-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-29062: Add standalone module for packaging the Metastore #5923
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
Conversation
|
I wanted to do the same, but wasn't sure on the naming. Should we still build the Also, we are lacking the HMS client. There is already a patch with +1, but for some reason it wasn't merged.
|
|
@dengzhhu653, do you plan to include this in branch-4.1? |
Actually the Metastore needs the client, one example is the
yes, it's applicable |
|
Upon the latest master, there is an error on the I think it might due to HIVE-29028, I haven't seen such error before |
|
It seems that the HS2 process requires the loading of tez-dag.jar, but currently in our HS2, only tez-api.jar is loaded. |
|
@dengzhhu653, @zhangbutao, IcebergTableOptimizer is called from the HMS Initiator process, it shouldn't require the tez. I think we should revert |
true, still i've raised a PR for the HIVE-20189: Separate metastore client code into its own module to have a clear separation:
WDYT? |
| <directory>${project.basedir}</directory> | ||
| <directory>${project.parent.basedir}</directory> | ||
| <includes> | ||
| <include>DEV-README</include> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these files are not present in the standalone-metastore, should I include them?
| <includes> | ||
| <include>DEV-README</include> | ||
| <include>README*</include> | ||
| <include>LICENSE*</include> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we include these as well? note, was able to find CHANGELOG
<include>LICENSE</include>
<include>NOTICE</include>
<include>CHANGELOG</include>
<include>RELEASE_NOTES.txt</include>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deniskuzZ
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, some minor things
deniskuzZ
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, pending tests
|



What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?