aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
Date Mon, 26 Jan 2015 18:49:58 GMT


> On Jan. 26, 2015, 10:14 a.m., Aurora ReviewBot wrote:
> > Master (7ba6226) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > :assemble
> > :compileJmhJavawarning: Supported source version 'RELEASE_6' from annotation processor
'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source '1.7'
> > 1 warning
> > 
> > :processJmhResources UP-TO-DATE
> > :jmhClasses
> > :checkstyleJmh
> > :jsHint
> > :checkstyleMain
> > :compileTestJava
> > :processTestResources
> > :testClasses
> > :checkstyleTest
> > :findbugsJmh
> > :findbugsMain
> > :findbugsTest
> > :licenseJmh UP-TO-DATE
> > :licenseMain UP-TO-DATE
> > :licenseTest UP-TO-DATE
> > :license UP-TO-DATE
> > :pmdMain
> > :test
> > :jacocoTestReport
> > Coverage report generated: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
> > :analyzeReport
> > Instruction coverage of 0.8917038316949588 exceeds minimum coverage of 0.89.
> > :analyzeReport FAILED
> > 
> > FAILURE: Build failed with an exception.
> > 
> > * What went wrong:
> > Execution failed for task ':analyzeReport'.
> > > Branch coverage is 0.8343558282208589, but must be greater than 0.835
> > 
> > * Try:
> > Run with --stacktrace option to get the stack trace. Run with --info or --debug
option to get more log output.
> > 
> > BUILD FAILED
> > 
> > Total time: 3 mins 23.668 secs
> > 
> > 
> > I will refresh this build result if you post a review containing "@ReviewBot retry"
> 
> Bill Farner wrote:
>     > Branch coverage is 0.8343558282208589, but must be greater than 0.835
>     
>     Please ignore this failure, as you obviously did not affect branch coverage.  Looking
into this at https://issues.apache.org/jira/browse/AURORA-1060.

Ugh, the above comment was intended for a different review as this one could affect branch
coverage.  Bug stands, though, as i don't repro the branch check failure with this diff.


- Bill


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


On Jan. 25, 2015, 8:10 p.m., Florian Pfeiffer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30010/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2015, 8:10 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-184
>     https://issues.apache.org/jira/browse/AURORA-184
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
> 
> This is the first step for AURORA-184, that removes the default host&rack limit constraints.
> The second step that's still missing would be to add s.th. like "--default-constraints"
as start parameter to the scheduler. 
> 
> AURORA-174 could probably be closed with this?(since the rack limit constraint can be
configured in the .aurora file)
> 
> I can't really estimate the effect of my changes in StorageBackfillTest&SchedulerThriftInterfaceTest,
please have a closer look at the changes I did there.
> 
> Since this is also my first code submit, comments about codestyle&other bad habbits
are very appreciated.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 5dfbcf1f6de716502a28f7da33a095968eb8420e

>   src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
92ba45033ada8114349c435316c9681395aea706 
> 
> Diff: https://reviews.apache.org/r/30010/diff/
> 
> 
> Testing
> -------
> 
> Added test for ConfigurationManager.hasName 
> Added test testNoHostAndRackConstraintsAdded, that checks if the constraints are present
> Tested on vagrant devcluster to see if constraints are also gone in "real life"
> 
> 
> Thanks,
> 
> Florian Pfeiffer
> 
>


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