avro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jeff Hammerbacher" <jeff.hammerbac...@gmail.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:55:48 GMT


> On 2010-07-07 16:51:01, Philip Zeyliger wrote:
> > 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.

Well, considering that Avro RPC is under active development, I think now is the time to make
massively breaking changes. The discussion on this ticket seems to indicate that the changes
are for the better, so I vote for making them now while we still can.

To that end, see also https://issues.apache.org/jira/browse/AVRO-594


> On 2010-07-07 16:51:01, Philip Zeyliger wrote:
> > trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java, line 65
> > <http://review.hbase.org/r/281/diff/1/?file=2195#file2195line65>
> >
> >     catch (Exception e) is poor form, typically.  It's usually better to explicitly
catch the type of exception that Jetty's start throws.

Jetty's start() throws Exception: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/component/LifeCycle.html#start%28%29


> On 2010-07-07 16:51:01, Philip Zeyliger wrote:
> > trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java, line 60
> > <http://review.hbase.org/r/281/diff/1/?file=2195#file2195line60>
> >
> >     I would add to the java doc that this throws AvroRuntimeException.

Will do.


- Jeff


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


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