> 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 > >
