You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2014/05/29 06:32:53 UTC
Review Request 22007: Fix logic error when populating task resources.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22007/
-----------------------------------------------------------
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-483
https://issues.apache.org/jira/browse/AURORA-483
Repository: aurora
Description
-------
Brought Resources class up to 100% test coverage to mitigate future regressions.
Also fixed a bug i spotted in MesosTaskFactory, which we apparently have not tripped yet.
Source of this specific regression is here:
commit a8fa267f03e2fa36039eb014884858074f7b7575
Author: Bill Farner <wf...@apache.org>
Date: Tue May 27 11:25:02 2014 -0700
Enable some additional PMD rules, and fix sources to satisfy them.
Reviewed at https://reviews.apache.org/r/21849/
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b/src/main/java/org/apache/aurora/scheduler/configuration/Resource
index 5a38479..9ee2a85 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
@@ -112,7 +112,7 @@ public class Resources {
.add(Resources.makeMesosResource(CPUS, numCpus))
.add(Resources.makeMesosResource(DISK_MB, disk.as(Data.MB)))
.add(Resources.makeMesosResource(RAM_MB, ram.as(Data.MB)));
- if (selectedPorts.size() > 0) {
+ if (selectedPorts.isEmpty()) {
resourceBuilder.add(Resources.makeMesosRangeResource(Resources.PORTS, selectedPorts));
}
Diffs
-----
src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java f48ca51791ac7e4343d577db94807ff17e50a9af
src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 9ee2a8522b5ef3a3b7555928972cdd6e94aec43e
src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java fa601e1a70e84c7cfac20409b7aaa85b3a1d1816
src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java 08788a2f1ba9ffe5fc09c89bd840b2afe68eb227
Diff: https://reviews.apache.org/r/22007/diff/
Testing
-------
./gradlew build
Thanks,
Bill Farner
Re: Review Request 22007: Fix logic error when populating task resources.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22007/#review44262
-----------------------------------------------------------
Ship it!
Ship It!
- Maxim Khutornenko
On May 29, 2014, 4:32 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22007/
> -----------------------------------------------------------
>
> (Updated May 29, 2014, 4:32 a.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-483
> https://issues.apache.org/jira/browse/AURORA-483
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Brought Resources class up to 100% test coverage to mitigate future regressions.
> Also fixed a bug i spotted in MesosTaskFactory, which we apparently have not tripped yet.
>
> Source of this specific regression is here:
> commit a8fa267f03e2fa36039eb014884858074f7b7575
> Author: Bill Farner <wf...@apache.org>
> Date: Tue May 27 11:25:02 2014 -0700
>
> Enable some additional PMD rules, and fix sources to satisfy them.
>
> Reviewed at https://reviews.apache.org/r/21849/
>
> diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b/src/main/java/org/apache/aurora/scheduler/configuration/Resource
> index 5a38479..9ee2a85 100644
> --- a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
> +++ b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
> @@ -112,7 +112,7 @@ public class Resources {
> .add(Resources.makeMesosResource(CPUS, numCpus))
> .add(Resources.makeMesosResource(DISK_MB, disk.as(Data.MB)))
> .add(Resources.makeMesosResource(RAM_MB, ram.as(Data.MB)));
> - if (selectedPorts.size() > 0) {
> + if (selectedPorts.isEmpty()) {
> resourceBuilder.add(Resources.makeMesosRangeResource(Resources.PORTS, selectedPorts));
> }
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java f48ca51791ac7e4343d577db94807ff17e50a9af
> src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 9ee2a8522b5ef3a3b7555928972cdd6e94aec43e
> src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java fa601e1a70e84c7cfac20409b7aaa85b3a1d1816
> src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java 08788a2f1ba9ffe5fc09c89bd840b2afe68eb227
>
> Diff: https://reviews.apache.org/r/22007/diff/
>
>
> Testing
> -------
>
> ./gradlew build
>
>
> Thanks,
>
> Bill Farner
>
>