> On July 30, 2014, 7:02 p.m., Bill Havanki wrote:
> > server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/MasterResource.java,
> >  line 56
> > <https://reviews.apache.org/r/23988/diff/1/?file=643638#file643638line56>
> >
> >     To be more RESTful, consider returning an HTTP error response. (500, 
> > maybe a 404 or 204?)

Yeah, good call. I think I wrote this before I figured out the 
WebApplicationException(Status) stuff. There are probably more places to use 
response codes, too. Need to put more thought into the semantics behind the 
methods.


> On July 30, 2014, 7:02 p.m., Bill Havanki wrote:
> > server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/health/AccumuloHealthCheck.java,
> >  lines 88-94
> > <https://reviews.apache.org/r/23988/diff/1/?file=643635#file643635line88>
> >
> >     Is there a way to provide more detail about the failure to read the 
> > metadata table? I might like to know specifically that the check timed out 
> > vs. some other error.

Absolutely. I'll be making an umbrella JIRA later today (after chatting with 
Christopher). I'll try to address this too.


> On July 30, 2014, 7:02 p.m., Bill Havanki wrote:
> > server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TableInformation.java,
> >  line 62
> > <https://reviews.apache.org/r/23988/diff/1/?file=643631#file643631line62>
> >
> >     Use negative numbers to indicate no information, perhaps?

I think we really want these fields to be required. I need to look at the 
creator of this data (the remote side of the thrift call) that we're wrapping 
to see if there are ever cases in which this data might be absent.


- Josh


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


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

Reply via email to