[
https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12886187#action_12886187
]
Philip Zeyliger commented on AVRO-544:
--------------------------------------
>From reviewboard:
Jeff Hammerbacher:
{quote}
> 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
{quote}
Scott Carey:
{quote}
ReviewBoard is awesome. I'll need to figure out how to use it sometime.
Comments inline:
On Jul 7, 2010, at 4:51 PM, Philip Zeyliger wrote:
>
> -----------------------------------------------------------
> 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.
>
Yeah, that is a concern. This is definitely an improvement though.
>
> 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.
>
"@throws AvroRuntimeException if the underlying Jetty server throws any
exception while starting." ?
>
>
> 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.
>
True, but Jetty's start() signature is:
void org.mortbay.component.AbstractLifeCycle.start() throws Exception
So my only caution with this patch is the backwards-incompatible change.
Luckily it is not subtle. I'm fine with the change but am not a major user of
HttpServer. This does likely imply a change for other Servers down the road
too.
{quote}
> 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.
{quote}
> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
> Key: AVRO-544
> URL: https://issues.apache.org/jira/browse/AVRO-544
> Project: Avro
> Issue Type: New Feature
> Components: java
> Reporter: Jeff Hammerbacher
> Assignee: Jeff Hammerbacher
> Fix For: 1.4.0
>
> Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch,
> AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer:
> http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
