On 03/06/2014 04:33 PM, Tom White wrote:
>
> In 
> kite-data/kite-data-hcatalog/src/main/java/org/kitesdk/data/hcatalog/HCatalog.java:
>
> > @@ -50,7 +50,11 @@ public HCatalog(Configuration conf) {
> >    public Table getTable(String dbName, String tableName) {
> >      Table table;
> >      try {
> > -      table = HCatUtil.getTable(client, dbName, tableName);
> > +      synchronized(client) {
> > +        table = HCatUtil.getTable(client, dbName, tableName);
> > +      }
> > +    } catch (RuntimeException e) {
> > +        throw new RuntimeException("Hive metastore exception", e);
>
> Any reason this is needed given we already catch Exception? (Also, 
> it's only been added in a few cases - e.g. tableExists doesn't catch 
> RuntimeException.)
>

I added it because findbugs complained about it (see link), and since it 
is already done in other places I just followed suit.

http://findbugs.sourceforge.net/bugDescriptions.html#REC_CATCH_EXCEPTION

I have to admit I really don't understand why findbugs does not complain 
when the synchronized(){...} is omitted though

> If we do need it it might be better to make this a DatasetException so 
> we know it came from Kite.
>

You decide. See the other `catch (RuntimeException)` in this class.

> —
> Reply to this email directly or view it on GitHub 
> <https://github.com/kite-sdk/kite/pull/29/files#r10346026>.
>