> On May 14, 2018, 7:26 p.m., Aurora ReviewBot wrote:
> > Master (805a53f) is red with this patch.
> > ./build-support/jenkins/build.sh
> >
> > :compileJmhJavaNote:
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
> > uses or overrides a deprecated API.
> > Note: Recompile with -Xlint:deprecation for details.
> >
> > :processJmhResources NO-SOURCE
> > :jmhClasses
> > :checkstyleJmh
> > :checkstyleMain[ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:789:7:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:790:9:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:802:7:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:805:14:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:808:14:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java:816:9:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java:123:9:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java:127:16:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java:20:8:
> > Unused import - com.google.common.collect.FluentIterable. [UnusedImports]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java:22:
> > Wrong order for 'java.util.stream.Collectors' import. Order should be:
> > java, javax, scala, com, net, org. Each group should be separated
> > by a single blank line. [ImportOrder]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:19:
> > Wrong order for 'java.util.List' import. Order should be: java, javax,
> > scala, com, net, org. Each group should be separated by a single
> > blank line. [ImportOrder]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:25:
> > Wrong order for 'java.util.stream.Collectors' import. Order should be:
> > java, javax, scala, com, net, org. Each group should be separated
> > by a single blank line. [ImportOrder]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:70:11:
> > Redundant 'final' modifier. [RedundantModifier]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:70:44:
> > '{' is not preceded with whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:100:11:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:109:49:
> > '-' is not preceded with whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:109:50:
> > '-' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:117:7:
> > 'if' is not followed by whitespace. [WhitespaceAround]
> > [ant:checkstyle] [ERROR]
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java:127:
> > Line is longer than 100 characters (found 101). [LineLength]
> > FAILED
> >
> > FAILURE: Build failed with an exception.
> >
> > * What went wrong:
> > Execution failed for task ':checkstyleMain'.
> > > Checkstyle rule violations were found. See the report at:
> > > file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html
> >
> > * Try:
> > Run with --stacktrace option to get the stack trace. Run with --info or
> > --debug option to get more log output.
> >
> > * Get more help at https://help.gradle.org
> >
> > BUILD FAILED in 5m 13s
> > 32 actionable tasks: 26 executed, 6 up-to-date
> >
> >
> > I will refresh this build result if you post a review containing
> > "@ReviewBot retry"
>
> Santhosh Kumar Shanmugham wrote:
> Can you clean up the style issues?
>
> Also, will this be configurable via the pystachio config?
Definitely, just wanted to get the latest version out to get feedback on the
Thrift Schema changes. This will indeed be configurable via pystachio, just
wanted to settle on the Thrift Schema before making those changes.
- Renan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66192/#review203093
-----------------------------------------------------------
On May 14, 2018, 7:19 p.m., Renan DelValle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66192/
> -----------------------------------------------------------
>
> (Updated May 14, 2018, 7:19 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Jordan Ly, Santhosh Kumar
> Shanmugham, and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding support for variable group sizes when executing an update.
>
> Design doc for this change is here:
> https://docs.google.com/document/d/1xGk4ueH8YlmJCk6hQJh85u4to4M1VQD0l630IOchvgY/edit#heading=h.lg3hty82f5cz
>
> I opted for the path of least resistance with regards to the Thrift changes
> as I didn't see any benefit in making the larger changes required to make the
> interfaces a bit more flexible.
>
> Requesting feedback on these changes and the approach from the community
> before I proceed.
>
> Tests will be added after the community approves of the direciton and
> approach.
>
> Note to reviewers: Changes made in ActiveLimitedStrategy.java were made to
> move towards getting rid of FluentIterable. I figured since I was touching
> that code, it wouldn't hurt to test the Java 8 equivalent of it. I can get
> rid of the change here and make it in a separate patch if desired.
>
>
> Diffs
> -----
>
> api/src/main/thrift/org/apache/aurora/gen/api.thrift
> ef754e32172e7490a47a13e7b526f243ffa3efeb
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
> 9fc0416086dd3eb2e2f4e8f659da59fcdea2b22b
> src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java
> 3992aa77fc305adc390a4aaeb1d3939d6241ddbd
>
> src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java
> 855ea9c20788b51695b7eff5ac0970f0d52a9546
>
> src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java
> PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/66192/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Renan DelValle
>
>
