[jira] [Comment Edited] (HIVE-17812) Move remaining classes that HiveMetaStore depends on

["Vihang Karajgaonkar (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HIVE-17812?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16216009#comment-16216009 ] 

Vihang Karajgaonkar edited comment on HIVE-17812 at 10/23/17 11:04 PM:
-----------------------------------------------------------------------

Hi [~alangates] I did a first pass on the patch and added some comments in the github review. I went over your comment regarding backwards incompatibility above after I submitted the review comments so some of them might seem redundant.

bq. Is this avoidable? Yes, I could move events and listeners in a bigger patch along with HiveMetaStore. However, other changes are going to break listeners and events anyway. Namely, the change from HiveConf -> Conf (which is not avoidable). Also, if we do split the metastore into a separate TLP in the future it will change class names, which will also obviously impact implementations of listener. We should look into what it will take to build a shim that would support existing listeners. This would need to live in the metastore module rather than standalone-metastore, since it will need to reference HiveConf.

The change from {{HMSHandler}} to {{IHMSHandler}} in the events makes sense to me from design point of view. But I think it is unclear if it is worth breaking backwards compatibility. I am not sure I understand why it would have broken the compatibility because of HiveConf -> Conf change. Do the events use conf object? Based on what I looked (I didn't look at all the events but few sample of the important events) I didn't see conf being used. If we change the class names, we might still be able to provide some shims layer to preserve compatibility with respect to the public API if we don't modify the method signatures. What do you think?



was (Author: vihangk1):
Hi [~alangates] I did a first pass on the patch and added some comments in the github review. I went over your comment regarding backwards incompatibility above after I submitted the review comments so some of them might seem redundant.

bq. Is this avoidable? Yes, I could move events and listeners in a bigger patch along with HiveMetaStore. However, other changes are going to break listeners and events anyway. Namely, the change from HiveConf -> Conf (which is not avoidable). Also, if we do split the metastore into a separate TLP in the future it will change class names, which will also obviously impact implementations of listener.
We should look into what it will take to build a shim that would support existing listeners. This would need to live in the metastore module rather than standalone-metastore, since it will need to reference HiveConf.

The change from {{HMSHandler}} to {{IHMSHandler}} in the events makes sense to me from design point of view. But I think it is unclear if it is worth breaking backwards compatibility. I am not sure I understand why it would have broken the compatibility because of HiveConf -> Conf change. Do the events use conf object? Based on what I looked (I didn't look at all the events but few sample of the important events) I didn't see conf being used. If we change the class names, we might still be able to provide some shims layer to preserve compatibility with respect to the public API if we don't modify the method signatures. What do you think?


> Move remaining classes that HiveMetaStore depends on 
> -----------------------------------------------------
>
>                 Key: HIVE-17812
>                 URL: https://issues.apache.org/jira/browse/HIVE-17812
>             Project: Hive
>          Issue Type: Sub-task
>          Components: Metastore
>            Reporter: Alan Gates
>            Assignee: Alan Gates
>              Labels: pull-request-available
>         Attachments: HIVE-17812.2.patch, HIVE-17812.3.patch, HIVE-17812.patch
>
>
> There are several remaining pieces that need moved before we can move HiveMetaStore itself.  These include NotificationListener and implementations, Events, AlterHandler, and a few other miscellaneous pieces.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)