accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 23988: Use Dropwizard to create a proper REST monitor service
Date Mon, 28 Jul 2014 17:53:11 GMT


> On July 28, 2014, 5:35 p.m., Christopher Tubbs wrote:
> > assemble/bin/start-all.sh, lines 62-63
> > <https://reviews.apache.org/r/23988/diff/1/?file=643615#file643615line62>
> >
> >     Since this component is optional, it seems like it should not be started automatically
with the other standard services (the ones users have come to expect).

That depends. If the intent is still for the monitor (as we know it now) to eventually use
this service (make the monitor a consumer of this rest service), I think it makes sense to
just start the process and get people used to it.


> On July 28, 2014, 5:35 p.m., Christopher Tubbs wrote:
> > pom.xml, line 129
> > <https://reviews.apache.org/r/23988/diff/1/?file=643620#file643620line129>
> >
> >     Are we introducing incompatibilities that will make it harder for users to use
newer versions of Jetty? Why the version drop?

Dropwizard bundles this all for us, and thus, to use it, requires the drop to make it work.
Users presently don't care what version of Jetty is used. The point of the library isn't to
make a war file that can be deployed but to make a singular artifact that run the service.
If you're referring to system integrators (like what you're doing for fedora), the point is
still moot because you don't need jetty installed on the system elsewhere. The dependencies
are encapsulated within the monitor-rest artifact.


> On July 28, 2014, 5:35 p.m., Christopher Tubbs wrote:
> > server/monitor-rest/pom.xml, lines 84-96
> > <https://reviews.apache.org/r/23988/diff/1/?file=643622#file643622line84>
> >
> >     The rest server uses flot?

Probably just a copy-paste leftover from when I created the monitor-rest module.


> On July 28, 2014, 5:35 p.m., Christopher Tubbs wrote:
> > server/monitor-rest/pom.xml, lines 98-131
> > <https://reviews.apache.org/r/23988/diff/1/?file=643622#file643622line98>
> >
> >     Please don't shade by default in the build. It creates a nightmare for pom dependency
resolution. We should not be shipping shaded binary artifacts in a release, or deploying them
to maven central in a release.

That's the whole point of Dropwizard. I'd recommend you read into it - https://dropwizard.github.io/dropwizard/getting-started.html,
specifically https://dropwizard.github.io/dropwizard/getting-started.html#building-fat-jars.


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23988/#review48869
-----------------------------------------------------------


On July 28, 2014, 4:55 p.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23988/
> -----------------------------------------------------------
> 
> (Updated July 28, 2014, 4:55 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3005
>     https://issues.apache.org/jira/browse/ACCUMULO-3005
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Creates a proper REST service using Dropwizard, with the intent to eventually replace
the existing monitor's "data" component. Copies most of the functionality (sans the log-forwarding)
into a standalone application. Returns data as JSON and tries to separate logic into consumable
pieces. Still uses the Monitor class for most Thrift interactions.
> 
> 
> Diffs
> -----
> 
>   assemble/bin/accumulo 727a4c8 
>   assemble/bin/start-all.sh cebbd8c 
>   assemble/bin/stop-all.sh 4bf06c0 
>   assemble/bin/stop-server.sh 52696af 
>   assemble/conf/templates/accumulo-site.xml 08c905b 
>   assemble/pom.xml 89a3747 
>   pom.xml ba6693d 
>   server/monitor-rest/.gitignore PRE-CREATION 
>   server/monitor-rest/pom.xml PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/MonitorApplication.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/MonitorConfiguration.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/GarbageCollection.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/GarbageCollectorCycle.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/GarbageCollectorStatus.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/LogEvent.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/RecoveryStatusInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/ReplicationInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TableInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TabletServerInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TabletServerTableInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TabletServerWithTableInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/health/AccumuloHealthCheck.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/GarbageCollectorResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/LogResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/MasterResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/ProblemsResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/ReplicationResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/StatisticsOverTimeResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/StatisticsResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/TablesResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/TabletServerResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/resources/accumulo-monitor-rest.yaml PRE-CREATION 
>   server/monitor-rest/src/test/resources/log4j.properties PRE-CREATION 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java 268516c 
> 
> Diff: https://reviews.apache.org/r/23988/diff/
> 
> 
> Testing
> -------
> 
> Tested against a single-node Accumulo instance. While this code is much easier to test,
I haven't written new unit tests yet.
> 
> 
> Thanks,
> 
> Josh Elser
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message