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:[email protected]>
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:[email protected]>
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
