Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

2017-10-05 Thread Jie Yu 

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

Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.


Bugs: MESOS-7990
https://issues.apache.org/jira/browse/MESOS-7990


Repository: mesos


Description
---

This patch added the support for systemd hierarchy in LinuxLauncher.
It created the same cgroup layout under the systemd hierarchy (if
systemd is enabled) as that in the freezer hierarchy.

This can give us a bunch of benefits:
1) systemd-cgls can list mesos container processes.
2) systemd-cgtop can show stats for mesos containers.
3) Avoid the pid migration issue described in MESOS-3352.

For example:

```
[jie@core-dev ~]$ systemd-cgls
|-1 /usr/lib/systemd/systemd --system --deserialize 20
|-mesos
|  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
|  |-31737 /usr/libexec/mesos/mesos-containerizer launch
|  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
|  |-mesos
| |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
|   |-31791 /usr/libexec/mesos/mesos-containerizer launch
|   |-31793 sleep 1000
```


Diffs
-

  src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
  src/slave/containerizer/mesos/linux_launcher.cpp 
554c598e2a7a53aede9d8761740d8efceb4a7e39 


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


Testing
---

sudo make check

I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
issue. Didn't observe it after applying the patch.


Thanks,

Jie Yu

Review Request 62799: Fixed an issue for the I/O switchboard process lifetime.

2017-10-05 Thread Jie Yu 

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

Review request for mesos, Joseph Wu and Kevin Klues.


Bugs: MESOS-8056
https://issues.apache.org/jira/browse/MESOS-8056


Repository: mesos


Description
---

We expect the I/O switchboard process to last across agent restarts
(similar to log rotate process or executor processes). Therefore, we
should put it into 'mesos_executor.slice' like others.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
63d3cf80d8af6347c70fa6b7e2fd824faa0c7e3f 


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


Testing
---

sudo make check


Thanks,

Jie Yu

Review Request 62798: Added named cgroup hierarchy support.

2017-10-05 Thread Jie Yu 

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

Review request for mesos, James Peach and Joris Van Remoortere.


Bugs: MESOS-7990
https://issues.apache.org/jira/browse/MESOS-7990


Repository: mesos


Description
---

This patch add a helper to get the cgroup associated with the given
pid for a named cgroup hierarchy.


Diffs
-

  src/linux/cgroups.hpp 154e5b6c1adef4f4e77f1747c56dfd83a9967ed4 
  src/linux/cgroups.cpp 0a7bb89567d4b3ecee7c4f9a259368fb4eae7f34 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 60628: Enable fetcher_tests.cpp unit test module on Windows platform.

2017-10-05 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 60292.

Failed command: `python.exe .\support\apply-reviews.py -n -r 60292`

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

Relevant logs:

- 
[apply-review-60292-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628/logs/apply-review-60292-stdout.log):

```
error: patch failed: 3rdparty/stout/include/stout/stringify.hpp:57
error: 3rdparty/stout/include/stout/stringify.hpp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 6, 2017, 4:04 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> ---
> 
> (Updated Oct. 6, 2017, 4:04 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document that some FetcherTest tests won't work on Windows platform.
>   Tests for tar, gzip, and such won't be working on Windows for
>   the time being. Thoughts are to provide this capability to the
>   Fetcher in a cross-platform manner via a programmatic code library
>   rather than Linux-specific command line tools (tar, gzip, etc).
> 
>   In the short term, zip and unzip will work since PowerShell can
>   support that natively.
> 
> Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
> Test FetcherTest.InvalidUser can't work on Windows for now.
> Fix test FetcherTest.NonExistingFile on Windows platform.
> Fix test FetcherTest.AbsolutePath on Windows platform.
> Fix test FetcherTest.RelativeFilePath on Windows platform.
> Fix test FetcherTest.OSNetUriTest on Windows platform.
> Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
> Fix test FetcherTest.FileLocalhostURI on Windows platform.
> Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
> Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
> Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
> Fix test FetcherTest.UseCustomOutputFile on Windows platform.
> 
> 
> Diffs
> -
> 
>   src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b 
>   src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d 
>   src/slave/containerizer/fetcher.cpp 
> ba5b0979aa427c5b3dbacf39661d6027da13e0d6 
>   src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
>   src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5 
> 
> 
> Diff: https://reviews.apache.org/r/60628/diff/2/
> 
> 
> Testing
> ---
> 
> See upstream.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 60628: Enable fetcher_tests.cpp unit test module on Windows platform.

2017-10-05 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 60292.

Failed command: `python.exe .\support\apply-reviews.py -n -r 60292`

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

Relevant logs:

- 
[apply-review-60292-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628/logs/apply-review-60292-stdout.log):

```
error: patch failed: 3rdparty/stout/include/stout/stringify.hpp:57
error: 3rdparty/stout/include/stout/stringify.hpp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 6, 2017, 4:04 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> ---
> 
> (Updated Oct. 6, 2017, 4:04 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document that some FetcherTest tests won't work on Windows platform.
>   Tests for tar, gzip, and such won't be working on Windows for
>   the time being. Thoughts are to provide this capability to the
>   Fetcher in a cross-platform manner via a programmatic code library
>   rather than Linux-specific command line tools (tar, gzip, etc).
> 
>   In the short term, zip and unzip will work since PowerShell can
>   support that natively.
> 
> Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
> Test FetcherTest.InvalidUser can't work on Windows for now.
> Fix test FetcherTest.NonExistingFile on Windows platform.
> Fix test FetcherTest.AbsolutePath on Windows platform.
> Fix test FetcherTest.RelativeFilePath on Windows platform.
> Fix test FetcherTest.OSNetUriTest on Windows platform.
> Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
> Fix test FetcherTest.FileLocalhostURI on Windows platform.
> Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
> Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
> Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
> Fix test FetcherTest.UseCustomOutputFile on Windows platform.
> 
> 
> Diffs
> -
> 
>   src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b 
>   src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d 
>   src/slave/containerizer/fetcher.cpp 
> ba5b0979aa427c5b3dbacf39661d6027da13e0d6 
>   src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
>   src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5 
> 
> 
> Diff: https://reviews.apache.org/r/60628/diff/2/
> 
> 
> Testing
> ---
> 
> See upstream.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 60625: Normalize file separation characters on Windows when building path.

2017-10-05 Thread Jeff Coffler 

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

(Updated Oct. 6, 2017, 4:05 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
---

Normalize file separation characters on Windows when building path.


Diffs (updated)
-

  3rdparty/stout/include/stout/path.hpp 
6ee3a44cd6a878fe383aa68df40b82857b93d0b4 


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

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


Testing
---

See upstream


Thanks,

Jeff Coffler

Re: Review Request 60620: Modify os::write to write binary files on Windows.

2017-10-05 Thread Jeff Coffler 

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

(Updated Oct. 6, 2017, 4:04 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
---

Modify os::write to write binary files on Windows.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/write.hpp 
9ff749f209e6dd6ca3695907108a029c9a2b4f05 


Diff: https://reviews.apache.org/r/60620/diff/3/

Changes: https://reviews.apache.org/r/60620/diff/2-3/


Testing
---

Built successfully on both Linux (with autotools and cmake) and Windows (with 
cmake).

Ran stout-tests and mesos-tests successfully on both Windows and Linux.


Thanks,

Jeff Coffler

Re: Review Request 60626: Eliminate os::shell calls from HDFS for Windows compatibility.

2017-10-05 Thread Jeff Coffler 

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

(Updated Oct. 6, 2017, 4:04 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
---

Eliminate os::shell calls from HDFS for Windows compatibility.


Diffs (updated)
-

  src/hdfs/hdfs.cpp 2c95a5ea43a4289e1168c527b9ccc35690a751a4 


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

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


Testing
---

See upstream


Thanks,

Jeff Coffler

Re: Review Request 60623: Convert "file://" URI handling to use new uri function.

2017-10-05 Thread Jeff Coffler 

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

(Updated Oct. 6, 2017, 4:03 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Summary (updated)
-

Convert "file://" URI handling to use new uri function.


Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description (updated)
---

Convert "file://" URI handling to use new uri function.


Diffs (updated)
-

  src/tests/credentials_tests.cpp 7eb5abd21a1be35d4739c4928cb922f028cfc9a7 
  src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5 
  src/tests/master_contender_detector_tests.cpp 
f499136c0b981072af5bc8accf2238b7ba4bf430 
  src/tests/script.cpp 8d40e01da005cb05e7804f0b3975e3e0edb8f3bd 


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

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


Testing
---

See upstream


Thanks,

Jeff Coffler

Re: Review Request 60624: Enable HDFS compilation and associated tests.

2017-10-05 Thread Jeff Coffler 

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

(Updated Oct. 6, 2017, 4:04 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
---

Note that tests are disabled for Windows due to dependence on 'sh'
shell.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515 


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

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


Testing
---

See upstream


Thanks,

Jeff Coffler

Re: Review Request 60628: Enable fetcher_tests.cpp unit test module on Windows platform.

2017-10-05 Thread Jeff Coffler 

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

(Updated Oct. 6, 2017, 4:04 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
---

Document that some FetcherTest tests won't work on Windows platform.
  Tests for tar, gzip, and such won't be working on Windows for
  the time being. Thoughts are to provide this capability to the
  Fetcher in a cross-platform manner via a programmatic code library
  rather than Linux-specific command line tools (tar, gzip, etc).

  In the short term, zip and unzip will work since PowerShell can
  support that natively.

Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
Test FetcherTest.InvalidUser can't work on Windows for now.
Fix test FetcherTest.NonExistingFile on Windows platform.
Fix test FetcherTest.AbsolutePath on Windows platform.
Fix test FetcherTest.RelativeFilePath on Windows platform.
Fix test FetcherTest.OSNetUriTest on Windows platform.
Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
Fix test FetcherTest.FileLocalhostURI on Windows platform.
Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
Fix test FetcherTest.UseCustomOutputFile on Windows platform.


Diffs (updated)
-

  src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b 
  src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d 
  src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5 


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

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


Testing
---

See upstream.


Thanks,

Jeff Coffler

Re: Review Request 60622: Add new stout functions for path normalizaiton and URI conversion.

2017-10-05 Thread Jeff Coffler 

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

(Updated Oct. 6, 2017, 4:03 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Summary (updated)
-

Add new stout functions for path normalizaiton and URI conversion.


Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description (updated)
---

Add new stout function: path::normalize (normalize a filename).
Add new stout function: uri::uriFromFilename (generate URI from
  filename).


Diffs (updated)
-

  3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5 
  3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751 
  3rdparty/stout/include/stout/path.hpp 
6ee3a44cd6a878fe383aa68df40b82857b93d0b4 
  3rdparty/stout/include/stout/uri.hpp PRE-CREATION 
  3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd 
  3rdparty/stout/tests/path_tests.cpp f8c14d5aefe0b49adb778da784143a328c96183d 
  3rdparty/stout/tests/uri_tests.cpp PRE-CREATION 


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

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


Testing
---

See upstream


Thanks,

Jeff Coffler

Re: Review Request 60624: Enable HDFS compilation and associated tests.

2017-10-05 Thread Jeff Coffler 


> On Aug. 15, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 267 (patched)
> > 
> >
> > This one is straight-forward. On Windows, there is no "executable 
> > permissions" so we can simply ignore this logic. With a note of course as 
> > to why.

This needs to be generally revisited, and is on the list. This is a much bigger 
change, I want to handle it all at once.


> On Aug. 15, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 487 (patched)
> > 
> >
> > I'm not suggesting to do this _right now_. But for `chown`, Windows has 
> > a mappable concept of taking (probably recursive) ownership of the 
> > directory as the specified user. So we can handle this, we'll just need to 
> > implement it.

Permissions is an issue throughout Mesos today, and should be looked at at that 
time.


> On Aug. 15, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 564 (patched)
> > 
> >
> > I think we can do the same thing as `os::su` with Windows user 
> > impersonation. It's a mappable concept.

It's only mappable when we get the permissions thing worked out, and we can run 
on something other than an admistrative account.


> On Aug. 15, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/hdfs_tests.cpp
> > Lines 56 (patched)
> > 
> >
> > Like above, we can safely ignore setting execution permission on 
> > Windows. My reasoning is that the concept maps to nothing. So doing nothing 
> > is appropriate I think.

Again, this sort of thing is in many places (including other modules), and I'd 
like to fix that issue all at once.


- Jeff


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


On July 3, 2017, 7:32 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> ---
> 
> (Updated July 3, 2017, 7:32 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Note that tests are disabled for Windows due to dependence on 'sh'
> shell.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 51b67428f823454665db695ba70a0dc896a7595c 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/tests/CMakeLists.txt 9c0acaf43f451dbc9ba5077529a36aa4cef40c34 
>   src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515 
> 
> 
> Diff: https://reviews.apache.org/r/60624/diff/1/
> 
> 
> Testing
> ---
> 
> See upstream
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 60621: Add new stout capability: os::copyfile.

2017-10-05 Thread Jeff Coffler 

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

(Updated Oct. 6, 2017, 4:03 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
---

Add new stout capability: os::copyfile.


Diffs (updated)
-

  3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5 
  3rdparty/stout/include/Makefile.am bdd3e9908ebfc682458a3babc34cbee36ad3f751 
  3rdparty/stout/include/stout/os/copyfile.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/copyfile.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/copyfile.hpp PRE-CREATION 
  3rdparty/stout/tests/CMakeLists.txt 6e5773f1e03671de7ac007caf49edd0f1cd7aedd 
  3rdparty/stout/tests/os/copyfile_tests.cpp PRE-CREATION 


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

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


Testing
---

See upstream

Note that Joe made some changes to this, I ended up taking his changes as is.


Thanks,

Jeff Coffler

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

2017-10-05 Thread Zhitao Li 

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




src/slave/containerizer/mesos/containerizer.cpp
Lines 1186-1202 (patched)


@gilbert, I realized one thing: should this be moved to or duplicated in 
`containerizer::prepare()`, after the `provisionInfo` is returned from 
`provisioner::provision()` finishes? In the chained callback there, 
`container->config` could be updated one more time if some rootfs is 
provisioned.


- Zhitao Li


On Sept. 26, 2017, 7:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Sept. 26, 2017, 7:09 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> cc23b4d91be16fc95a131c09d07378b801e34d6f 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4d5dc13f363f5d8886983d7dd06a5cecc177c345 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 62774: Added a test `ROOT_DOCKER_NoTransitionFromKillingToFinished`.

2017-10-05 Thread Vinod Kone 

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 4, 2017, 3:48 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62774/
> ---
> 
> (Updated Oct. 4, 2017, 3:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7975
> https://issues.apache.org/jira/browse/MESOS-7975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ROOT_DOCKER_NoTransitionFromKillingToFinished`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 45f0d1dbc5b21b174cd9974664299d466129df01 
> 
> 
> Diff: https://reviews.apache.org/r/62774/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 62327: Checked TASK_KILLED in the test `ROOT_INTERNET_CURL_PortMapper`.

2017-10-05 Thread Vinod Kone 

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 14, 2017, 10:45 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62327/
> ---
> 
> (Updated Sept. 14, 2017, 10:45 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7975
> https://issues.apache.org/jira/browse/MESOS-7975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Checked TASK_KILLED in the test `ROOT_INTERNET_CURL_PortMapper`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 636695f368d1c8acd9ed4ccfd1a449f375af8282 
> 
> 
> Diff: https://reviews.apache.org/r/62327/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 62214: Added JavaScript linter.

2017-10-05 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [62333, 62214]

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

- Mesos Reviewbot


On Sept. 29, 2017, 1:28 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Sept. 29, 2017, 1:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This linter runs when changes on a JavaScript file are being committed.
> The linter used is ESLint with a configuration based on our current JS
> code base. The linter and its dependencies (i.e. Node.js) are installed
> in a environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/5/
> 
> 
> Testing
> ---
> 
> Following this commit, I have tried to commit a change on a JavaScript file 
> and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62287: Added Log::Reader::catchup() method.

2017-10-05 Thread Ilya Pronin 

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

(Updated Oct. 5, 2017, 1:05 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated description.


Bugs: MESOS-7973
https://issues.apache.org/jira/browse/MESOS-7973


Repository: mesos


Description (updated)
---

The new method catches-up positions missing in the local non-leading
replica to allow safe eventually consistent reads from it.


Diffs
-

  include/mesos/log/log.hpp d92d7a0b982c1c4bc0fdedde19fe012a6523a224 
  src/log/log.hpp 24481aaae145301e7a8ea1c91b1c16e48ded4843 
  src/log/log.cpp dcb66f7dd5da5059925b7b731e83f7320f645030 
  src/tests/log_tests.cpp f9f9400c901152779ae0ebfe74cf8f7aac1d3396 


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


Testing
---

Added a test that verifies that it's possible to catch-up the lagging replica 
using the `Reader` and successfully read entries that were caught-up. Ran `make 
check`.


Thanks,

Ilya Pronin

Re: Review Request 62733: Organized configuration documentation.

2017-10-05 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 62729.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62729`

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

Relevant logs:

- 
[apply-review-62729-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62733/logs/apply-review-62729-stdout.log):

```
error: patch failed: 3rdparty/cmake/External.cmake:19
error: 3rdparty/cmake/External.cmake: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 5, 2017, 7:01 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62733/
> ---
> 
> (Updated Oct. 5, 2017, 7:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jeff Coffler, Greg Mann, John 
> Kordich, James Peach, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This splits `docs/configuration.md` into the `docs/configuration`
> folder. The documentation had grown too large, and so with
> `configuration.md` becoming a table of contents, the discoverability of
> the various runtime and build configurations is improved.
> 
> 
> Diffs
> -
> 
>   docs/agent-recovery.md f8f9cae87a7d6f4888b44014f1e907a2d0f3cfc7 
>   docs/authorization.md da51e8bad274ad9c480246782d9d810198713e6d 
>   docs/cmake.md PRE-CREATION 
>   docs/configuration.md e1fd9f75179b272c3cae3dd5be5e38f269044df5 
>   docs/configuration/agent.md PRE-CREATION 
>   docs/configuration/autotools.md PRE-CREATION 
>   docs/configuration/cmake.md PRE-CREATION 
>   docs/configuration/libprocess.md PRE-CREATION 
>   docs/configuration/master-and-agent.md PRE-CREATION 
>   docs/configuration/master.md PRE-CREATION 
>   docs/container-image.md 74bf7bed5dfa5f5ecf8c384286001f68d33c376c 
>   docs/docker-containerizer.md bab84dc2b0ce104b3ec59aaf0ef800b418a6517c 
>   docs/fetcher.md 55d243540785a66640184a4775db5f6db240ab4f 
>   docs/home.md f9b35e3e8f9af024a58760c345931d73a83654ff 
>   docs/logging.md 84d54bf85d5d0e6da737c1f3854001f0c1e52409 
>   docs/sandbox.md 233eb1c90f33813f2b4673773823ec181dd25c9a 
> 
> 
> Diff: https://reviews.apache.org/r/62733/diff/2/
> 
> 
> Testing
> ---
> 
> Verified the generated site works properly with the refactor.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 62733: Organized configuration documentation.

2017-10-05 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 62729.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62729`

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

Relevant logs:

- 
[apply-review-62729-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62733/logs/apply-review-62729-stdout.log):

```
error: patch failed: 3rdparty/cmake/External.cmake:19
error: 3rdparty/cmake/External.cmake: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 5, 2017, 7:01 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62733/
> ---
> 
> (Updated Oct. 5, 2017, 7:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jeff Coffler, Greg Mann, John 
> Kordich, James Peach, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This splits `docs/configuration.md` into the `docs/configuration`
> folder. The documentation had grown too large, and so with
> `configuration.md` becoming a table of contents, the discoverability of
> the various runtime and build configurations is improved.
> 
> 
> Diffs
> -
> 
>   docs/agent-recovery.md f8f9cae87a7d6f4888b44014f1e907a2d0f3cfc7 
>   docs/authorization.md da51e8bad274ad9c480246782d9d810198713e6d 
>   docs/cmake.md PRE-CREATION 
>   docs/configuration.md e1fd9f75179b272c3cae3dd5be5e38f269044df5 
>   docs/configuration/agent.md PRE-CREATION 
>   docs/configuration/autotools.md PRE-CREATION 
>   docs/configuration/cmake.md PRE-CREATION 
>   docs/configuration/libprocess.md PRE-CREATION 
>   docs/configuration/master-and-agent.md PRE-CREATION 
>   docs/configuration/master.md PRE-CREATION 
>   docs/container-image.md 74bf7bed5dfa5f5ecf8c384286001f68d33c376c 
>   docs/docker-containerizer.md bab84dc2b0ce104b3ec59aaf0ef800b418a6517c 
>   docs/fetcher.md 55d243540785a66640184a4775db5f6db240ab4f 
>   docs/home.md f9b35e3e8f9af024a58760c345931d73a83654ff 
>   docs/logging.md 84d54bf85d5d0e6da737c1f3854001f0c1e52409 
>   docs/sandbox.md 233eb1c90f33813f2b4673773823ec181dd25c9a 
> 
> 
> Diff: https://reviews.apache.org/r/62733/diff/2/
> 
> 
> Testing
> ---
> 
> Verified the generated site works properly with the refactor.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 62733: Organized configuration documentation.

2017-10-05 Thread Andrew Schwartzmeyer 

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

(Updated Oct. 5, 2017, 12:01 p.m.)


Review request for mesos, Benjamin Bannier, Jeff Coffler, Greg Mann, John 
Kordich, James Peach, Joseph Wu, and Li Li.


Changes
---

Rebased.


Repository: mesos


Description
---

This splits `docs/configuration.md` into the `docs/configuration`
folder. The documentation had grown too large, and so with
`configuration.md` becoming a table of contents, the discoverability of
the various runtime and build configurations is improved.


Diffs (updated)
-

  docs/agent-recovery.md f8f9cae87a7d6f4888b44014f1e907a2d0f3cfc7 
  docs/authorization.md da51e8bad274ad9c480246782d9d810198713e6d 
  docs/cmake.md PRE-CREATION 
  docs/configuration.md e1fd9f75179b272c3cae3dd5be5e38f269044df5 
  docs/configuration/agent.md PRE-CREATION 
  docs/configuration/autotools.md PRE-CREATION 
  docs/configuration/cmake.md PRE-CREATION 
  docs/configuration/libprocess.md PRE-CREATION 
  docs/configuration/master-and-agent.md PRE-CREATION 
  docs/configuration/master.md PRE-CREATION 
  docs/container-image.md 74bf7bed5dfa5f5ecf8c384286001f68d33c376c 
  docs/docker-containerizer.md bab84dc2b0ce104b3ec59aaf0ef800b418a6517c 
  docs/fetcher.md 55d243540785a66640184a4775db5f6db240ab4f 
  docs/home.md f9b35e3e8f9af024a58760c345931d73a83654ff 
  docs/logging.md 84d54bf85d5d0e6da737c1f3854001f0c1e52409 
  docs/sandbox.md 233eb1c90f33813f2b4673773823ec181dd25c9a 


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

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


Testing
---

Verified the generated site works properly with the refactor.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 62732: Added CMake documentation.

2017-10-05 Thread Andrew Schwartzmeyer 


> On Oct. 2, 2017, 3:26 p.m., Joseph Wu wrote:
> > docs/cmake.md
> > Lines 42 (patched)
> > 
> >
> > Hm... The website's markdown generator is not super sophisticated, so 
> > I'm not sure if it will render this link correctly.
> > 
> > It's ok to go over 80 characters on a line if it's just a URL.
> 
> Andrew Schwartzmeyer wrote:
> Ha! It better work, this is in Daring Fireball's original Markdown spec 
> (i.e. it's not any "flavor's" extension). I'll bug Vinod and make sure, but 
> it really ought to work.
> 
> My problem with embedding URLs in the text is just that, it makes the 
> plaintext so much more difficult to read. But maybe that's just me.

Verified that this is no problem with the generated site.


- Andrew


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


On Oct. 5, 2017, noon, Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62732/
> ---
> 
> (Updated Oct. 5, 2017, noon)
> 
> 
> Review request for mesos, Benjamin Bannier, Jeff Coffler, Greg Mann, John 
> Kordich, James Peach, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-3107
> https://issues.apache.org/jira/browse/MESOS-3107
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a CMake documentation file with best practices, CMake By
> Example, and consolidates, fixes, and updates the CMake configuration
> options to the configuration documentation.
> 
> 
> Diffs
> -
> 
>   docs/cmake-examples.md PRE-CREATION 
>   docs/cmake.md PRE-CREATION 
>   docs/configuration-cmake.md 1b10221429a92eddff5733e1059172b6c190b5f3 
>   docs/configuration.md e1fd9f75179b272c3cae3dd5be5e38f269044df5 
>   docs/home.md f9b35e3e8f9af024a58760c345931d73a83654ff 
> 
> 
> Diff: https://reviews.apache.org/r/62732/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 62732: Added CMake documentation.

2017-10-05 Thread Andrew Schwartzmeyer 

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

(Updated Oct. 5, 2017, noon)


Review request for mesos, Benjamin Bannier, Jeff Coffler, Greg Mann, John 
Kordich, James Peach, Joseph Wu, and Li Li.


Changes
---

Addressed comments and added step-by-step for Java and SSL on Windows.


Bugs: MESOS-3107
https://issues.apache.org/jira/browse/MESOS-3107


Repository: mesos


Description
---

This adds a CMake documentation file with best practices, CMake By
Example, and consolidates, fixes, and updates the CMake configuration
options to the configuration documentation.


Diffs (updated)
-

  docs/cmake-examples.md PRE-CREATION 
  docs/cmake.md PRE-CREATION 
  docs/configuration-cmake.md 1b10221429a92eddff5733e1059172b6c190b5f3 
  docs/configuration.md e1fd9f75179b272c3cae3dd5be5e38f269044df5 
  docs/home.md f9b35e3e8f9af024a58760c345931d73a83654ff 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 62793: Updated list of files excluded by Python linter.

2017-10-05 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 62793 was successfully built and tested.

Reviews applied: `['62793']`

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

- Mesos Reviewbot Windows


On Oct. 5, 2017, 4:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62793/
> ---
> 
> (Updated Oct. 5, 2017, 4:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This make sure that only our Python files and
> no third party libraries are being linted by Pylint.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
> 
> 
> Diff: https://reviews.apache.org/r/62793/diff/1/
> 
> 
> Testing
> ---
> 
> After this commit I have modified 'support/cpplint.py' and checked that it 
> was not linted.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62793: Updated list of files excluded by Python linter.

2017-10-05 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 62793 was successfully built and tested.

Reviews applied: `['62793']`

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

- Mesos Reviewbot Windows


On Oct. 5, 2017, 4:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62793/
> ---
> 
> (Updated Oct. 5, 2017, 4:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This make sure that only our Python files and
> no third party libraries are being linted by Pylint.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
> 
> 
> Diff: https://reviews.apache.org/r/62793/diff/1/
> 
> 
> Testing
> ---
> 
> After this commit I have modified 'support/cpplint.py' and checked that it 
> was not linted.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62548: Reorganized and updated the contribution guidelines.

2017-10-05 Thread Andrew Schwartzmeyer 

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


Fix it, then Ship it!




This is fantastic, ship it!


docs/home.md
Lines 98-99 (patched)


Top of the list. +1



docs/newbie-guide.md
Line 37 (original), 26 (patched)


I think you can make the links to Getting Started relative links, like 
`[Getting Started](getting-started.md)`. The site generation takes care of 
making the link work, and this will avoid changes to the top-level domain (and 
me making comments like, "this should be HTTPS!" because everything should 
always be HTTPS).



docs/newbie-guide.md
Lines 39 (patched)


:D



docs/newbie-guide.md
Line 66 (original), 53 (patched)


I love the link to the YouTube video, that will help people out a lot I 
think.

I've tried to explain `libprocess` as "it's just like Erlang processes!" 
but that wasn't a great explanation if they hadn't used Erlang...



docs/submitting-a-patch.md
Line 2 (original), 2 (patched)


I think making this the advanced guide makes a ton more sense. +1


- Andrew Schwartzmeyer


On Sept. 25, 2017, 11:02 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62548/
> ---
> 
> (Updated Sept. 25, 2017, 11:02 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-564
> https://issues.apache.org/jira/browse/MESOS-564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reorganized and updated the contribution guidelines.
> 
> 
> Diffs
> -
> 
>   docs/home.md f9b35e3e8f9af024a58760c345931d73a83654ff 
>   docs/newbie-guide.md e9f2aaac1bb986120f34b1d006e3a2f5eb2779ff 
>   docs/reopening-reviews.md fe5046830bbdf28fcc2377ffa5792549920afbc8 
>   docs/reporting-a-bug.md a7e372c1f0d3a34a06244aecb1dfeef7356b8928 
>   docs/submitting-a-patch.md ffc6e561b8721b8849ef6025c15936ea712d3bfa 
>   site/source/community.html.md f56131fedb935ff695206948e16252c62ae0f36a 
> 
> 
> Diff: https://reviews.apache.org/r/62548/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 62793: Updated list of files excluded by Python linter.

2017-10-05 Thread Armand Grillet 

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

Review request for mesos, Alexander Rukletsov and Kevin Klues.


Repository: mesos


Description
---

This make sure that only our Python files and
no third party libraries are being linted by Pylint.


Diffs
-

  support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 


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


Testing
---

After this commit I have modified 'support/cpplint.py' and checked that it was 
not linted.


Thanks,

Armand Grillet

Re: Review Request 62788: Added support/ to the list of the linted directories.

2017-10-05 Thread Kevin Klues 

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


Ship it!




Ship It!

- Kevin Klues


On Oct. 5, 2017, 2:01 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62788/
> ---
> 
> (Updated Oct. 5, 2017, 2:01 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By adding the support directory to 'mesos-style.py', we make sure
> that all our support scripts follow the same code style that the
> rest of our Python codebase uses.
> 
> We add invalid-name to 'pylint.config' as all the Python files
> in support/ use dashes instead of underscores and we also add
> 'file-ignored' as we do not lint 'support/post-reviews.py' yet.
> 
> 
> Diffs
> -
> 
>   src/python/pylint.config 9f0361d160fa82038ccefcbc146e52f0af8f99e8 
>   support/mesos-style.py bab7f5e0699e4f8faa0ac973551cac5773d1b1fe 
>   support/post-reviews.py 5ecde40553c6b1ace9bfcf27a7268450b8bc2838 
> 
> 
> Diff: https://reviews.apache.org/r/62788/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 62788: Added support/ to the list of the linted directories.

2017-10-05 Thread Armand Grillet 

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

Review request for mesos and Kevin Klues.


Bugs: MESOS-6390
https://issues.apache.org/jira/browse/MESOS-6390


Repository: mesos


Description
---

By adding the support directory to 'mesos-style.py', we make sure
that all our support scripts follow the same code style that the
rest of our Python codebase uses.

We add invalid-name to 'pylint.config' as all the Python files
in support/ use dashes instead of underscores and we also add
'file-ignored' as we do not lint 'support/post-reviews.py' yet.


Diffs
-

  src/python/pylint.config 9f0361d160fa82038ccefcbc146e52f0af8f99e8 
  support/mesos-style.py bab7f5e0699e4f8faa0ac973551cac5773d1b1fe 
  support/post-reviews.py 5ecde40553c6b1ace9bfcf27a7268450b8bc2838 


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


Testing
---


Thanks,

Armand Grillet

Re: Review Request 62214: Added JavaScript linter.

2017-10-05 Thread Kevin Klues 

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




support/mesos-style.py
Lines 356-361 (patched)


This should already be handled by the `exclude_files` list and the 
`find_candidates()` function in the base class. Is it necessary to duplicate 
this logic here?



support/mesos-style.py
Lines 364 (patched)


I don't think this function will ever be called if len(source_paths) == 0 
(meaning this check is unnecessary). See the main() function from the base 
class.


- Kevin Klues


On Sept. 29, 2017, 1:28 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Sept. 29, 2017, 1:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This linter runs when changes on a JavaScript file are being committed.
> The linter used is ESLint with a configuration based on our current JS
> code base. The linter and its dependencies (i.e. Node.js) are installed
> in a environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/5/
> 
> 
> Testing
> ---
> 
> Following this commit, I have tried to commit a change on a JavaScript file 
> and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62214: Added JavaScript linter.

2017-10-05 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 62333.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62333`

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

Relevant logs:

- 
[apply-review-62333-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62214/logs/apply-review-62333-stdout.log):

```
error: patch failed: support/mesos-style.py:153
error: support/mesos-style.py: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 29, 2017, 6:28 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Sept. 29, 2017, 6:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This linter runs when changes on a JavaScript file are being committed.
> The linter used is ESLint with a configuration based on our current JS
> code base. The linter and its dependencies (i.e. Node.js) are installed
> in a environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/5/
> 
> 
> Testing
> ---
> 
> Following this commit, I have tried to commit a change on a JavaScript file 
> and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62214: Added JavaScript linter.

2017-10-05 Thread Mesos Reviewbot Windows 

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



FAIL: Failed to apply the dependent review: 62333.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62333`

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

Relevant logs:

- 
[apply-review-62333-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62214/logs/apply-review-62333-stdout.log):

```
error: patch failed: support/mesos-style.py:153
error: support/mesos-style.py: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 29, 2017, 1:28 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Sept. 29, 2017, 1:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This linter runs when changes on a JavaScript file are being committed.
> The linter used is ESLint with a configuration based on our current JS
> code base. The linter and its dependencies (i.e. Node.js) are installed
> in a environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/5/
> 
> 
> Testing
> ---
> 
> Following this commit, I have tried to commit a change on a JavaScript file 
> and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62777: Fixed build dependency for `protobuf_tests.proto`.

2017-10-05 Thread Benjamin Bannier 

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




3rdparty/stout/Makefile.am
Line 218 (original), 219 (patched)


Hmm, adding these files to the normal sources incorrectly models that some 
other translation units depend on the generated headers (namely 
`tests/protobuf_tests.cpp`). This can leads builds starting the compile of 
`protobuf_tests.cpp` before `protobuf_tests.pb.h` is generated (can happen in 
both parallel and sequential builds). This is one aspect that adding the 
generated files to `BUILT_SOURCES` prevents.

How about we explicitly depend on `BUNDLED_DEPS` in the protobuf generation 
rule, and still add the generated proto files to `BUILT_SOURCES`?

%.pb.cc %.pb.h: tests/%.proto $(BUNDLED_DEPS)
   ...


- Benjamin Bannier


On Oct. 4, 2017, 8:53 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62777/
> ---
> 
> (Updated Oct. 4, 2017, 8:53 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8052
> https://issues.apache.org/jira/browse/MESOS-8052
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The protobuf hpp and cpp files for `protobuf_tests.proto` should be
> built after `BUNDLED_DEPS`, not within it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5 
> 
> 
> Diff: https://reviews.apache.org/r/62777/diff/1/
> 
> 
> Testing
> ---
> 
> make -j4 check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 62333: Added helper functions for linters using a virtual environment.

2017-10-05 Thread Kevin Klues 

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


Ship it!




Ship It!

- Kevin Klues


On Sept. 29, 2017, 1:23 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62333/
> ---
> 
> (Updated Sept. 29, 2017, 1:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used by the Python linter and
> will be used by the Javascript linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62333/diff/3/
> 
> 
> Testing
> ---
> 
> Manual updates of `.py` and `.js` files then test commits to see if the 
> linters before and afer removing their `.virtualenv` were still working as 
> expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62333: Added helper functions for linters using a virtual environment.

2017-10-05 Thread Kevin Klues 


> On Oct. 5, 2017, 1:31 p.m., Kevin Klues wrote:
> > Ship It!

I made a change locally to remove the `super(Pytlint, self)` calls where 
unnecessary.


- Kevin


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


On Sept. 29, 2017, 1:23 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62333/
> ---
> 
> (Updated Sept. 29, 2017, 1:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used by the Python linter and
> will be used by the Javascript linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62333/diff/3/
> 
> 
> Testing
> ---
> 
> Manual updates of `.py` and `.js` files then test commits to see if the 
> linters before and afer removing their `.virtualenv` were still working as 
> expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62777: Fixed build dependency for `protobuf_tests.proto`.

2017-10-05 Thread Benjamin Bannier 

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




3rdparty/stout/Makefile.am
Line 122 (original), 122 (patched)


We generally assume GNU make so we can produce multiple targets from a 
single rule invocation, but for some reason this failed here, 
https://reviews.apache.org/r/62777/#last-review.

The GNU extension we rely on makes sure to invoke multi-target rules 
exactly once; otherwise rules producing multiple output files could be invoked 
`num_target` times in parallel where the different processes race on the same 
output files, possibly producing junk.

I tested this with a pattern rule

%.pb.cc %pb.h: tests/%.proto
$(PROTOC) $(PROTOCFLAGS) --cpp_out=$(builddir) $^

with looped `make` invocations like

$ rm protobuf_tests.pb.cc protobuf_tests.pb.h && make -j 
stout_tests-protobuf_tests.pb.o

The form in this patch seems to produce multiple `protoc` invocations 
pretty often while the one suggested above worked fine in the limited number of 
tests I ran.


- Benjamin Bannier


On Oct. 4, 2017, 8:53 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62777/
> ---
> 
> (Updated Oct. 4, 2017, 8:53 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-8052
> https://issues.apache.org/jira/browse/MESOS-8052
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The protobuf hpp and cpp files for `protobuf_tests.proto` should be
> built after `BUNDLED_DEPS`, not within it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am 4386017acd6ca465be3f735460c11d50b440ccc5 
> 
> 
> Diff: https://reviews.apache.org/r/62777/diff/1/
> 
> 
> Testing
> ---
> 
> make -j4 check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>