[
https://issues.apache.org/jira/browse/FLUME-721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13088064#comment-13088064
]
[email protected] commented on FLUME-721:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1604/#review1577
-----------------------------------------------------------
Ship it!
We now expose Jetty dependencies (and enforce them) on consumers of
InternalHttpServer. We've further deviated from normal war deployment
complicating things for developers. Once again, we need to modify the core of
Flume to add / remove / update its web applications which completely defeats
the purpose of the original refactoring. The correct way to solve the missing
log view and stacks servlet errors was to just create two wars (or even one)
with the applications and drop it into the webapps directory; none of that
works anymore. I disagree with the approach entirely but I'm positive it works
as advertised. If we were going to just rewrite this back to StatusHttpServer,
we should have just reverted the InternalHttpServer patch (because that's what
this and the last change effectively do).
flume-core/src/main/java/com/cloudera/flume/agent/FlumeNode.java
<https://reviews.apache.org/r/1604/#comment3556>
The whole point of InternalHttpServer was to eliminate Jetty references
from FlumeNode.
flume-core/src/main/java/com/cloudera/flume/agent/FlumeNode.java
<https://reviews.apache.org/r/1604/#comment3557>
A nicer way to handle this would be with dependencies injection. This just
hard codes the binding here (rather than in InternalHttpServer) so it doesn't
buy us anything in terms of abstraction or control.
flume-core/src/main/java/com/cloudera/flume/master/FlumeMaster.java
<https://reviews.apache.org/r/1604/#comment3558>
A clean way to do this would be to configure a factory and have it return
configured instances of InternalHttpServer. What's missing is a DI container to
externalize the wiring of contexts -> http server -> flume node.
flume-core/src/main/java/com/cloudera/util/InternalHttpServer.java
<https://reviews.apache.org/r/1604/#comment3560>
By removing this we've effectively moved back out of the normal deployment
model. I don't understand why we can't adhere to conventions.
flume-core/src/main/java/com/cloudera/util/InternalHttpServer.java
<https://reviews.apache.org/r/1604/#comment3559>
Further exposure of Jetty dependency.
- Eric
On 2011-08-19 22:07:34, jmhsieh wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1604/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-08-19 22:07:34)
bq.
bq.
bq. Review request for Flume, Arvind Prabhakar and Eric Sammer.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This was previously posted at https://review.cloudera.org/r/1894/ and
pushed because of timeout. The patch actually was later then reverted because
it had some problems with imports. Since the previous patch had timed-out as
opposed to being actually reviewed, I'm reposting for review.
bq.
bq.
bq. FLUME-721: Webapps 'autofindport' feature does not work
bq.
bq. This refactors the internal http server so that context are created by
a callback object.
bq.
bq.
bq. This addresses bug flume-721.
bq. https://issues.apache.org/jira/browse/flume-721
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. flume-core/src/main/java/com/cloudera/flume/agent/FlumeNode.java b8f2b67
bq. flume-core/src/main/java/com/cloudera/flume/master/FlumeMaster.java
5c99c43
bq. flume-core/src/main/java/com/cloudera/util/HttpServerTestUtils.java
PRE-CREATION
bq. flume-core/src/main/java/com/cloudera/util/InternalHttpServer.java
1f41460
bq. flume-core/src/main/java/com/cloudera/util/StatusHttpServer.java 58c0ece
bq. flume-core/src/test/java/com/cloudera/flume/agent/TestNodeJersey.java
2625a99
bq. flume-core/src/test/java/com/cloudera/flume/master/TestMasterJersey.java
146cd33
bq. flume-core/src/test/java/com/cloudera/util/InternalHttpServerTest.java
b6a4cdd
bq. flume-core/src/test/java/com/cloudera/util/TestStatusHttpServer.java
ac47ac5
bq. flume-node-web/src/test/java/com/cloudera/flume/agent/TestBootstrap.java
8d241dd
bq.
bq. Diff: https://reviews.apache.org/r/1604/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. ran tests, they passed.
bq.
bq.
bq. Thanks,
bq.
bq. jmhsieh
bq.
bq.
> Webapps 'autofindport' feature does not work
> --------------------------------------------
>
> Key: FLUME-721
> URL: https://issues.apache.org/jira/browse/FLUME-721
> Project: Flume
> Issue Type: Bug
> Affects Versions: v0.9.4
> Reporter: Jonathan Hsieh
> Assignee: Jonathan Hsieh
> Priority: Critical
> Fix For: v0.9.5
>
> Attachments:
> 0001-FLUME-721-Webapps-autofindport-feature-does-not-work.patch,
> 0001-FLUME-721-Webapps-autofindport-feature-does-not-work.patch
>
>
> This is a regression in the upgrade and changes to the webapps from 0.9.3 to
> 0.9.4. In previous versions the default was to enable an option that
> automatically increment http server port in order if the desired port was had
> something bound to it. This made it easier to run and test multiple physical
> nodes on a single machine.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
