What is the "official status" of these APIs? Were they ever documented and advertised as public? Or clients were always using them at their own risk? Were there any commitment to maintain compatibility of these APIs?
Which is considered "more official" - the Thrift APIs or IMetastore ones? Which ones should be used by 3-rd party applications (or should any of these be used at all?) - Alex On Thu, Dec 14, 2017 at 4:56 AM, Peter Vary <pv...@cloudera.com> wrote: > Hi, > > Is it acceptable to introduce backward incompatible changes in the Thrift > interface in the 3.0.0. release? > If we want to handle the validation on the server side - so every Thrift > user can benefit from the changes - then we have to add new exceptions to > the throws clause on the method declared on the Thrift interface as well. > For example: > --- a/standalone-metastore/src/main/thrift/hive_metastore.thrift > +++ b/standalone-metastore/src/main/thrift/hive_metastore.thrift > @@ -1653,15 +1653,16 @@ service ThriftHiveMetastore extends > fb303.FacebookService > 4:NoSuchObjectException o4) > > void drop_function(1:string dbName, 2:string funcName) > - throws (1:NoSuchObjectException o1, 2:MetaException o3) > + throws (1:NoSuchObjectException o1, 2:InvalidObjectException o2, > 3:MetaException o3) > > void alter_function(1:string dbName, 2:string funcName, 3:Function > newFunc) > - throws (1:InvalidOperationException o1, 2:MetaException o2) > + throws (1:AlreadyExistsException o1, 2:InvalidObjectException o2, > 3:NoSuchObjectException o3, > + 4:MetaException o4) > > list<string> get_functions(1:string dbName, 2:string pattern) > - throws (1:MetaException o1) > + throws (1:MetaException o1, 2:InvalidObjectException o2) > Function get_function(1:string dbName, 2:string funcName) > - throws (1:MetaException o1, 2:NoSuchObjectException o2) > + throws (1:MetaException o1, 2:NoSuchObjectException o2, > 3:InvalidObjectException o3) > > Thanks, > Peter > > > On Dec 13, 2017, at 1:02 PM, Peter Vary <pv...@cloudera.com> wrote: > > > > Hi Team, > > > > Since the work begin to separate the HMS to a standalone project we > thought that it would be useful the create extensive API tests for the > public APIs of the new project. > > We started to create tests using the IMetaStoreClient interface > implementations and found that not surprisingly the happy paths are working > as expected, but there are some gaps in the exception handling. Most of the > enhancements affect the Thrift API as well. > > We went through the following methods: > > Functions > > Indexes > > Tables > > Databases > > We also plan to comb through at least the Partition related methods too. > > > > The possible enhancements we found could be grouped to the following > categories: > > Bugs: For example IMetaStoreClient.drop/alter/createFunction with null > function name might throw NullPointerException exception - this is clearly > a bug which should be solved: > > Embedded MetaStore throws NullPointerException > > Remote MetaStore client throws TTransportException > > Sub-optimal error handling: For example IMetaStoreClient.alterFunction > will not check if the new function is already exist, and tries to insert it > anyway. After 10 tries it throws a MetaException where the exception text > is "Update of object [...] failed : java.sql. > SQLIntegrityConstraintViolationException [...]". Fixing this could be > done without interface change, but following the logic of the other methods > on the interface AlreadyExistsException should be thrown. > > Inconsistent exception handling: Different methods will throw different > exceptions for similar errors. This makes the interface hard to understand, > hard to document and maintain. For example: > > Calling IMetaStoreClient.createTable with nonexistent database name will > throw InvalidObjectException > > Calling IMetaStoreClient.createFunction with nonexistent database name > database will throw NoSuchObjectException > > There are some cases when the Embedded MetaStore handles error > differently than the Remote MetaStore. For example: > IMetaStoreClient.dropTable with "null" as a database: > > Embedded MetaStore throws MetaException > > Remote MetaStore client throws TProtocolException > > > > Proposed changes: > > > > Fixing cases 1. and 2. is a simple bug fix - it could be done > independently > > Fixing cases 3. and 4. will change how the IMetaStoreClient and HMS > Thrift API works. For these we should review the IMetaStoreClient and HMS > Thrift API interface exception handling, to create consistent and easy to > follow rules for the possible exceptions. We propose to keep the current > exceptions, and only change when the given type of exceptions are thrown. > If we stick to this then the interface will be binary backward compatible > since currently every method defines TException as a throwable and every > exception is inherited from TException. I think we allowed to change this > since 3.0.0 is a major release. > > > > > > Do we agree with the general direction of these changes? > > > > Thanks, > > Peter > > > >
