accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Havanki" <bhava...@clouderagovt.com>
Subject Re: Review Request 19409: ACCUMULO-2497 - InstanceFactory
Date Wed, 19 Mar 2014 19:34:13 GMT


> On March 19, 2014, 12:52 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/InstanceFactory.java, line
37
> > <https://reviews.apache.org/r/19409/diff/1/?file=528072#file528072line37>
> >
> >     Is this exception class used? If not, it should be removed.
> 
> Bill Havanki wrote:
>     It's not, but I wanted to leave a way for future factory implementations to predictably
throw exceptions, at the factory level of abstraction (Effective Java 2nd ed. Item 61).
> 
> John Vines wrote:
>     I like that. Could we add a basic coding to the exception type to make various error
cases easier to add.
>     
>     And is there a particular reason you're making that exception type extend Runtime
instead of just Exception?
> 
> Sean Busbey wrote:
>     I presume it's because if it was not a RuntimeException it would have to go in the
method signature. Since the class isn't public, it would have to throw Exception, which is
not good.

The InstanceFactoryException class is in fact public because all members of interfaces are
public (JLS 6.6.1, also I had to double-check with javap :) ). I definitely intend for it
to be public.

I made it a RuntimeException because I judged that recovering from failing to get an Instance
would be unrecoverable. I could be wrong, and if so, I'm happy to make it checked.

John, is there any particular technique for adding a coding that you like? Numeric code? Enum?
I'll check around for any existing patterns for this sort of thing.


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19409/#review37731
-----------------------------------------------------------


On March 19, 2014, 12:44 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19409/
> -----------------------------------------------------------
> 
> (Updated March 19, 2014, 12:44 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2497
>     https://issues.apache.org/jira/browse/ACCUMULO-2497
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> To help in testing, this commit adds an InstanceFactory interface with some implementations.
> 
> To avoid changing the public API, some classes are in o.a.a.core.client.impl.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/InstanceFactory.java PRE-CREATION

>   core/src/main/java/org/apache/accumulo/core/client/impl/ZooKeeperInstanceFactory.java
PRE-CREATION 
>   core/src/test/java/org/apache/accumulo/core/client/impl/ZooKeeperInstanceFactoryTest.java
PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstanceFactory.java
PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/client/HdfsZooInstanceFactoryTest.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/19409/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message