> On May 15, 2018, 4:26 a.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?
>
> Renan DelValle wrote:
> 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.
>
> Santhosh Kumar Shanmugham wrote:
> Ack.
The changes in `api.thrift` look good to me!
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66192/#review203093
-----------------------------------------------------------
On May 15, 2018, 4:19 a.m., Renan DelValle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66192/
> -----------------------------------------------------------
>
> (Updated May 15, 2018, 4:19 a.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
>
>
