If we break the APIs, can we also do the API cleanup where we remove duplicate ones and change everything to use req/resp pattern?
On 17/12/14, 13:50, "Thejas Nair" <thejas.n...@gmail.com> wrote: >This direction looks good to me. >If the new exceptions are inheriting from TException the applications >would >still work. But would it still work if we old metastore client library is >used with a newer version of metastore server running with these changes ? > > >On Wed, Dec 13, 2017 at 4:02 AM, 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 >> >>
