If the application using the old API does not handle the original exceptions differently than the TExceptions then it should work as expected.
> On Dec 14, 2017, at 10:50 PM, 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 >> >>
