avro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger" <phi...@cloudera.com>
Subject Re: Review Request: AVRO-544: Allow the HttpServer to serve forever without a call to Thread.sleep()
Date Wed, 07 Jul 2010 23:51:01 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/281/#review320
-----------------------------------------------------------


The code, besides the catch (Exception e), looks fine. 

I do worry that this is a massively backwards-incompatible change.  Anyone who's set up an
Avro RPC server using HttpServer now needs to add a line to their code.  I can only think
of hacky ways around that.


trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java
<http://review.hbase.org/r/281/#comment1391>

    I would add to the java doc that this throws AvroRuntimeException.



trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java
<http://review.hbase.org/r/281/#comment1392>

    catch (Exception e) is poor form, typically.  It's usually better to explicitly catch
the type of exception that Jetty's start throws.


- Philip


On 2010-07-07 16:42:45, Jeff Hammerbacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/281/
> -----------------------------------------------------------
> 
> (Updated 2010-07-07 16:42:45)
> 
> 
> Review request for Avro.
> 
> 
> Summary
> -------
> 
> Adding start() to the Server interface and start() and join() to the HttpServer implementation.
Moved start() out of the HttpServer ctor and changed all places it was called in the codebase
to now grab an object and call start() on it.
> 
> 
> This addresses bug AVRO-544.
>     http://issues.apache.org/jira/browse/AVRO-544
> 
> 
> Diffs
> -----
> 
>   trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java 960285 
>   trunk/lang/java/src/java/org/apache/avro/ipc/Server.java 960285 
>   trunk/lang/java/src/java/org/apache/avro/tool/RpcReceiveTool.java 960285 
>   trunk/lang/java/src/test/java/org/apache/avro/TestBulkData.java 960285 
>   trunk/lang/java/src/test/java/org/apache/avro/TestProtocolHttp.java 960285 
>   trunk/lang/java/src/test/java/org/apache/avro/ipc/stats/StatsPluginOverhead.java 960285

>   trunk/lang/java/src/test/java/org/apache/avro/ipc/stats/TestStatsPluginAndServlet.java
960285 
> 
> Diff: http://review.hbase.org/r/281/diff
> 
> 
> Testing
> -------
> 
> ant test-java
> 
> 
> Thanks,
> 
> Jeff
> 
>


Mime
View raw message