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/02/07 17:02:22 UTC

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

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/resources/ResourceManager.java 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
  src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e 

Diff: https://reviews.apache.org/r/56395/diff/


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 Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56395/#review164737
-----------------------------------------------------------


Ship it!




This should remove the last internal user of the deprecated fields.

- Zameer Manji


On Feb. 7, 2017, 9:02 a.m., Nicol�s Donatucci wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 9:02 a.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/resources/ResourceManager.java 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> 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/#review166376
-----------------------------------------------------------




src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java (lines 344 - 345)
<https://reviews.apache.org/r/56395/#comment238307>

    I belive this will not work as expected if a resource type is missing. `getResource` throws `IllegalArgumentException` but users of the this class expect we throw `TaskDescriptionException`.
    
    My recommendation would be:
    * leave `getResource` in the `ThriftBackFill` class. It is scrape code that we will remove soon anyway.
    * Leverage the already existing `ResourceManager` methods here instead: `quantityOf(getTaskResources(config, CPUS))`


- Stephan Erb


On Feb. 7, 2017, 6:02 p.m., Nicol�s Donatucci wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 6:02 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/resources/ResourceManager.java 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> 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/#review164681
-----------------------------------------------------------



Master (5b7042c) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On Feb. 7, 2017, 5:02 p.m., Nicol�s Donatucci wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 5:02 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/resources/ResourceManager.java 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> 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/#review164679
-----------------------------------------------------------



@ReviewBot retry

- Stephan Erb


On Feb. 7, 2017, 6:02 p.m., Nicol�s Donatucci wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 6:02 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/resources/ResourceManager.java 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> 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


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, 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/#review164524
-----------------------------------------------------------



Master (cd4df56) is red with this patch.
  ./build-support/jenkins/build.sh

  Test coverage missing for org/apache/aurora/scheduler/mesos/TaskStatusStats$3
  Test coverage missing for org/apache/aurora/scheduler/mesos/TaskStatusStats$2
  Test coverage missing for org/apache/aurora/scheduler/mesos/TaskStatusStats$1
  Test coverage missing for org/apache/aurora/scheduler/mesos/MesosSchedulerImpl
  Test coverage missing for org/apache/aurora/scheduler/mesos/TaskStatusStats
  Test coverage missing for org/apache/aurora/scheduler/thrift/AuditMessages
  Test coverage missing for org/apache/aurora/scheduler/preemptor/Preemptor$PreemptorImpl
  Test coverage missing for org/apache/aurora/scheduler/preemptor/PendingTaskProcessor
  Test coverage missing for org/apache/aurora/scheduler/preemptor/ClusterStateImpl
  Test coverage missing for org/apache/aurora/scheduler/preemptor/PreemptionProposal
  Test coverage missing for org/apache/aurora/scheduler/preemptor/PendingTaskProcessor$1
  Test coverage missing for org/apache/aurora/scheduler/preemptor/PendingTaskProcessor$2
  Test coverage missing for org/apache/aurora/scheduler/preemptor/PendingTaskProcessor$3
  Test coverage missing for org/apache/aurora/scheduler/preemptor/PreemptorModule$PreemptorService
  Test coverage missing for org/apache/aurora/scheduler/events/PubsubEvent$DriverDisconnected
  Test coverage missing for org/apache/aurora/scheduler/events/WebhookModule
  Test coverage missing for org/apache/aurora/scheduler/events/PubsubEvent$TaskStatusReceived
  Test coverage missing for org/apache/aurora/scheduler/events/PubsubEventModule$RegisterSubscribers
  Test coverage missing for org/apache/aurora/scheduler/events/Webhook
  Test coverage missing for org/apache/aurora/scheduler/events/PubsubEvent$DriverRegistered
  Test coverage missing for org/apache/aurora/scheduler/events/WebhookInfo
  Test coverage missing for org/apache/aurora/scheduler/storage/log/EntrySerializer$EntrySerializerImpl$1
  Test coverage missing for org/apache/aurora/scheduler/storage/log/LogStorage$Settings
  Test coverage missing for org/apache/aurora/scheduler/storage/log/LogStorage$ScheduledExecutorSchedulingService
  Test coverage missing for org/apache/aurora/scheduler/storage/log/LogStorageModule
  Test coverage missing for org/apache/aurora/scheduler/storage/backup/BackupModule
  Test coverage missing for org/apache/aurora/scheduler/TaskVars
  Test coverage missing for org/apache/aurora/scheduler/SchedulerLifecycle$DefaultDelayedActions
  Test coverage missing for org/apache/aurora/scheduler/TierManager$TierManagerImpl$TierConfig
  Test coverage missing for org/apache/aurora/scheduler/TaskVars$Counter
  Test coverage missing for org/apache/aurora/scheduler/TaskVars$1
  Test coverage missing for org/apache/aurora/scheduler/storage/db/typehandlers/VolumeModeTypeHandler

* 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: 5 mins 46.569 secs


I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On Feb. 7, 2017, 5:02 p.m., Nicol�s Donatucci wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 5:02 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/resources/ResourceManager.java 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> 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>.

> On Feb. 14, 2017, 7:36 a.m., Reza Motamedi wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java, line 153
> > <https://reviews.apache.org/r/56395/diff/1/?file=1626711#file1626711line153>
> >
> >     I don't get why this method name has package path in it? Is it just moved from `ThriftBackFill`?

This is because in the ResourceManager there is the org.apache.mesos.v1.Protos.Resource already imported. So there are two Resource objects in scope.


- Nicol�s


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


On Feb. 7, 2017, 5:02 p.m., Nicol�s Donatucci wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 5:02 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/resources/ResourceManager.java 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> 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 Reza Motamedi <re...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56395/#review165471
-----------------------------------------------------------




src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java (line 153)
<https://reviews.apache.org/r/56395/#comment237319>

    I don't get why this method name has package path in it? Is it just moved from `ThriftBackFill`?


- Reza Motamedi


On Feb. 7, 2017, 5:02 p.m., Nicol�s Donatucci wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 5:02 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/resources/ResourceManager.java 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> Testing
> -------
> 
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Nicol�s Donatucci
> 
>