Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

2018-04-16 Thread Gaston Kleiman 

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


Ship it!




Ship It!

- Gaston Kleiman


On April 16, 2018, 9:51 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66615/
> ---
> 
> (Updated April 16, 2018, 9:51 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and 
> Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Docker containerizer tests were manually creating the `TaskInfo`
> for the tasks they wanted to launch. We can remove some of that
> boilerplate by adopting the `createTask` helper.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 847258daadf3c37d9071151616b18fc79d850ce8 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
> 
> 
> Diff: https://reviews.apache.org/r/66615/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 27).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

2018-04-16 Thread James Peach 


> On April 14, 2018, 12:25 a.m., Gaston Kleiman wrote:
> > Thanks for the cleanup!
> > 
> > I think we could get rid of quite some boilerplate with a helper that 
> > creates a `ContainerInfo::DockerInfo`.

Good point!


- James


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


On April 13, 2018, 11:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66615/
> ---
> 
> (Updated April 13, 2018, 11:57 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and 
> Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Docker containerizer tests were manually creating the `TaskInfo`
> for the tasks they wanted to launch. We can remove some of that
> boilerplate by adopting the `createTask` helper.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 847258daadf3c37d9071151616b18fc79d850ce8 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
> 
> 
> Diff: https://reviews.apache.org/r/66615/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 27).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

2018-04-16 Thread James Peach 

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

(Updated April 16, 2018, 4:51 p.m.)


Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and Zhitao 
Li.


Repository: mesos


Description
---

The Docker containerizer tests were manually creating the `TaskInfo`
for the tasks they wanted to launch. We can remove some of that
boilerplate by adopting the `createTask` helper.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
847258daadf3c37d9071151616b18fc79d850ce8 
  src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 


Diff: https://reviews.apache.org/r/66615/diff/2/

Changes: https://reviews.apache.org/r/66615/diff/1-2/


Testing
---

sudo make check (Fedora 27).


Thanks,

James Peach

Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

2018-04-13 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [66615]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 13, 2018, 11:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66615/
> ---
> 
> (Updated April 13, 2018, 11:57 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and 
> Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Docker containerizer tests were manually creating the `TaskInfo`
> for the tasks they wanted to launch. We can remove some of that
> boilerplate by adopting the `createTask` helper.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 847258daadf3c37d9071151616b18fc79d850ce8 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
> 
> 
> Diff: https://reviews.apache.org/r/66615/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 27).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

2018-04-13 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66615']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66615

Relevant logs:

- 
[mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66615/logs/mesos-tests-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(543):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(548):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(569):
 warning C4996: 'strcpy

Re: Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

2018-04-13 Thread Gaston Kleiman 

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



Thanks for the cleanup!

I think we could get rid of quite some boilerplate with a helper that creates a 
`ContainerInfo::DockerInfo`.


src/tests/containerizer/docker_containerizer_tests.cpp
Line 265 (original), 260 (patched)


Nit: I'd add an empty line above this one.

Here and in the other tests.


- Gaston Kleiman


On April 13, 2018, 4:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66615/
> ---
> 
> (Updated April 13, 2018, 4:57 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and 
> Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Docker containerizer tests were manually creating the `TaskInfo`
> for the tasks they wanted to launch. We can remove some of that
> boilerplate by adopting the `createTask` helper.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 847258daadf3c37d9071151616b18fc79d850ce8 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
> 
> 
> Diff: https://reviews.apache.org/r/66615/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 27).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 66615: Adopted `createTask` helper API in Docker containerizer tests.

2018-04-13 Thread James Peach 

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

Review request for mesos, Andrei Budnik, Gaston Kleiman, Greg Mann, and Zhitao 
Li.


Repository: mesos


Description
---

The Docker containerizer tests were manually creating the `TaskInfo`
for the tasks they wanted to launch. We can remove some of that
boilerplate by adopting the `createTask` helper.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
847258daadf3c37d9071151616b18fc79d850ce8 
  src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 


Diff: https://reviews.apache.org/r/66615/diff/1/


Testing
---

sudo make check (Fedora 27).


Thanks,

James Peach