[
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)
