hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alan Gates <alanfga...@gmail.com>
Subject Re: Need your opinion on HIVE-12916 to clean up metastore API
Date Thu, 28 Jan 2016 19:25:29 GMT
Are you proposing to do this in 2.0 or branch-1 or both?  In master we 
can put up with some backwards incompatibility for now.  But given that 
it's straightforward to maintain it (as Sergey outlines) even there it 
doesn't seem worth it.  So unless there's a very compelling reason (i.e. 
more than just code/API cleanup) we should maintain backwards 
compatibility.  And we have to maintain it in branch-1.

Alan.

> Sergey Shelukhin <mailto:sergey@hortonworks.com>
> January 25, 2016 at 17:30
> There are 2 separate issues.
>
> 1) Removing old APIs. There’s (almost) no impact from keeping the old
> function for compat if both old and new call the same internal method with
> slight argument variations, but there would be compact problems with
> removing them if someone is using it; metastore is used by many projects
> across the ecosystem, and I don’t know what functions they might be using.
> So I think we should keep the function. So far I’m -1 on removing (or
> breaking signature changes).
>
> 1a) We can deprecate the functions (in release notes, I presume; Thrift
> doesn’t support deprecation as far as I know), then remove them in next
> release, but I think we should do it in a systematic manner with entire
> metastore API, if there’s will to do the work.
>
> 2) Adding new arguments to old APIs.
> I’d rather that we add new API based on Request/Response pattern for such
> cases, those are easy to evolve. add_partitions vs add_partitions_req is
> one example.
> Then, as part of 1a, we can also remove the old APIs.
> I am -0 on that part of the JIRA, I guess it should be ok to (ab)use
> EnvironmentContext for additional parameters ;) As long as the change is
> backward compatible.
>
>
>
>
>
> Pengcheng Xiong <mailto:pxiong@apache.org>
> January 25, 2016 at 16:15
> Hi all,
>
> I was working on a new feature in HIVE-12730 and I was suggested by
> Ashutosh to put a configuration in EnvironmentContext. Then I found 
> that we
> have lots of cases where the configuration and EnvironmentContext both
> exist in separate functions in the API list. For example, we have
>
> public void alter_table(String dbname, String tbl_name, Table new_tbl)
> throws InvalidOperationException, MetaException,
> org.apache.thrift.TException;
>
> public void alter_table_with_environment_context(String dbname, String
> tbl_name, Table new_tbl, EnvironmentContext environment_context) throws
> InvalidOperationException, MetaException, org.apache.thrift.TException;
>
> public void alter_table_with_cascade(String dbname, String tbl_name, Table
> new_tbl, boolean cascade) throws InvalidOperationException, MetaException,
> org.apache.thrift.TException;
>
> My plan is to keep only
>
> public void alter_table_with_environment_context(String dbname, String
> tbl_name, Table new_tbl, EnvironmentContext environment_context)
> throws InvalidOperationException,
> MetaException, org.apache.thrift.TException;
>
> And then merge the other two into this single one at both client and
> server side. For example, put cascade in the environment_context at client
> side and then read it from server side. If we do so, then it is easy to
> extend in the future, for example, my current work in HIVE-12730.
>
> The patch is already there but Sergey has some concerns about backward
> compatibility. Could you please leave your comments/suggestions? Thanks a
> lot.
>
>
> Best
>
> Pengcheng
>

Mime
View raw message