You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Nicolás Donatucci <nd...@medallia.com> on 2017/03/13 15:53:08 UTC
Re: Review Request 56395: Change Resource Validation in
ConfigurationManager
so that it validates the Resource Set instead of deprecated fields
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56395/
-----------------------------------------------------------
(Updated March 13, 2017, 3:53 p.m.)
Review request for Aurora, Stephan Erb and Zameer Manji.
Changes
-------
I moved getReource back to the ThriftBackfill and instead used the QuantityOf method to do the validations. For this, the validations had to be done against the "config" instead of the backfilled "builder". This is ok given that the resource part of the backfill will be removed soon.
By doing this, I found a bug in the client. If "numGpus" was 0, then the whole task.resources would be the empty set. That was later corrected by the backfill. A small change to the client stops that from happening.
Repository: aurora
Description
-------
The Resource validation in ConfigurationManager is now done against the Resource set instead of the NumCpus, RamMb and DiskMb fields.
To do this I moved the GetResource method from the ThriftBackfill to the ResourceManager so it can be used in the ConfigurationManager without exposing it on the ThriftBackfill.
Related Issue: Aurora-1707
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 80f0aebd6de0c2d7c30a58ec09702d1c6d519787
src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e
src/main/python/apache/aurora/config/thrift.py 3539469d243638c0acd08bf0859d0ce858d8977c
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 5419d7edd83c14f155b105b87c5521d7a938e248
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 0cdd9829417bb3ef0215278a9458a3dd78a49a20
Diff: https://reviews.apache.org/r/56395/diff/2/
Changes: https://reviews.apache.org/r/56395/diff/1-2/
Testing
-------
src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
Thanks,
Nicol�s Donatucci
Re: Review Request 56395: Change Resource Validation in
ConfigurationManager
so that it validates the Resource Set instead of deprecated fields
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56395/#review168775
-----------------------------------------------------------
Master (a07b9ed) is red with this patch.
./build-support/jenkins/build.sh
:commons:processResources
:commons:classes
:commons:jar
:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74: Note: Wrote forwarder org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
:generateBuildProperties
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
: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 UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java:33:8: Unused import - org.apache.aurora.scheduler.resources.ResourceManager. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java:67: Line is longer than 100 characters (found 104). [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.
BUILD FAILED
Total time: 1 mins 32.452 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 13, 2017, 3:53 p.m., Nicol�s Donatucci wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
>
> (Updated March 13, 2017, 3:53 p.m.)
>
>
> Review request for Aurora, Stephan Erb and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The Resource validation in ConfigurationManager is now done against the Resource set instead of the NumCpus, RamMb and DiskMb fields.
>
> To do this I moved the GetResource method from the ThriftBackfill to the ResourceManager so it can be used in the ConfigurationManager without exposing it on the ThriftBackfill.
>
> Related Issue: Aurora-1707
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 80f0aebd6de0c2d7c30a58ec09702d1c6d519787
> src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e
> src/main/python/apache/aurora/config/thrift.py 3539469d243638c0acd08bf0859d0ce858d8977c
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 5419d7edd83c14f155b105b87c5521d7a938e248
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 0cdd9829417bb3ef0215278a9458a3dd78a49a20
>
>
> Diff: https://reviews.apache.org/r/56395/diff/2/
>
>
> Testing
> -------
>
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Nicol�s Donatucci
>
>
Re: Review Request 56395: Change Resource Validation in
ConfigurationManager
so that it validates the Resource Set instead of deprecated fields
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56395/#review168784
-----------------------------------------------------------
Ship it!
Master (a07b9ed) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 13, 2017, 4:46 p.m., Nicol�s Donatucci wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
>
> (Updated March 13, 2017, 4:46 p.m.)
>
>
> Review request for Aurora, Stephan Erb and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The Resource validation in ConfigurationManager is now done against the Resource set instead of the NumCpus, RamMb and DiskMb fields.
>
> To do this I moved the GetResource method from the ThriftBackfill to the ResourceManager so it can be used in the ConfigurationManager without exposing it on the ThriftBackfill.
>
> Related Issue: Aurora-1707
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 80f0aebd6de0c2d7c30a58ec09702d1c6d519787
> src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e
> src/main/python/apache/aurora/config/thrift.py 3539469d243638c0acd08bf0859d0ce858d8977c
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 5419d7edd83c14f155b105b87c5521d7a938e248
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 0cdd9829417bb3ef0215278a9458a3dd78a49a20
>
>
> Diff: https://reviews.apache.org/r/56395/diff/3/
>
>
> Testing
> -------
>
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Nicol�s Donatucci
>
>
Re: Review Request 56395: Change Resource Validation in
ConfigurationManager
so that it validates the Resource Set instead of deprecated fields
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56395/#review168828
-----------------------------------------------------------
Ship it!
Ship It!
- Stephan Erb
On March 13, 2017, 5:46 p.m., Nicol�s Donatucci wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
>
> (Updated March 13, 2017, 5:46 p.m.)
>
>
> Review request for Aurora, Stephan Erb and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The Resource validation in ConfigurationManager is now done against the Resource set instead of the NumCpus, RamMb and DiskMb fields.
>
> To do this I moved the GetResource method from the ThriftBackfill to the ResourceManager so it can be used in the ConfigurationManager without exposing it on the ThriftBackfill.
>
> Related Issue: Aurora-1707
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 80f0aebd6de0c2d7c30a58ec09702d1c6d519787
> src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e
> src/main/python/apache/aurora/config/thrift.py 3539469d243638c0acd08bf0859d0ce858d8977c
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 5419d7edd83c14f155b105b87c5521d7a938e248
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 0cdd9829417bb3ef0215278a9458a3dd78a49a20
>
>
> Diff: https://reviews.apache.org/r/56395/diff/3/
>
>
> Testing
> -------
>
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
>
>
> Thanks,
>
> Nicol�s Donatucci
>
>
Re: Review Request 56395: Change Resource Validation in
ConfigurationManager
so that it validates the Resource Set instead of deprecated fields
Posted by Nicolás Donatucci <nd...@medallia.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56395/
-----------------------------------------------------------
(Updated March 13, 2017, 4:46 p.m.)
Review request for Aurora, Stephan Erb and Zameer Manji.
Changes
-------
Code style fix.
Repository: aurora
Description
-------
The Resource validation in ConfigurationManager is now done against the Resource set instead of the NumCpus, RamMb and DiskMb fields.
To do this I moved the GetResource method from the ThriftBackfill to the ResourceManager so it can be used in the ConfigurationManager without exposing it on the ThriftBackfill.
Related Issue: Aurora-1707
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 80f0aebd6de0c2d7c30a58ec09702d1c6d519787
src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e
src/main/python/apache/aurora/config/thrift.py 3539469d243638c0acd08bf0859d0ce858d8977c
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 5419d7edd83c14f155b105b87c5521d7a938e248
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 0cdd9829417bb3ef0215278a9458a3dd78a49a20
Diff: https://reviews.apache.org/r/56395/diff/3/
Changes: https://reviews.apache.org/r/56395/diff/2-3/
Testing
-------
src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
Thanks,
Nicol�s Donatucci