Re: Review Request 41731: Removed docker puller flag.

[Joerg Schad]

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



src/slave/flags.cpp 


When changing these flags we should also update the documentation: 
http://mesos.apache.org/documentation/latest/configuration/



src/slave/flags.cpp (line 136)


Do I understand correctly that we change the default from "Pull from docker 
registry" to "pull from a local file location"? Wouldn't that impact users 
experimenting with Mesos? If this is not the case feel to drop.


- Joerg Schad


On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> ---
> 
> (Updated Dec. 27, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed docker puller flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f71b572a32443a6715acb4d3541aec60e0437b30 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> dd17acf6a6e5c306029198dbb2a7e2d059f87f75 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/41731/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 41657: Added user facing documentation for containerizers.

[Joerg Schad]

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



docs/containerizer.md (line 5)


Any reason for two blank lines?



docs/containerizer.md (line 19)


Should we mention the default value?



docs/containerizer.md (line 26)


What do you mean by "own native container technology"? Isn't that highly 
platform dependent?



docs/containerizer.md (line 30)


container technologies? Maybe something like: Mesos currently supports the 
following containerizer?



docs/containerizer.md (line 33)


Add Links to docs (maybe also/instead from sections below)?



docs/containerizer.md (lines 37 - 38)


Should this section maybe be after we introduce the other containerizer (I 
assume you went for alphabetical order)? Feel free to ignore if you disagree ...


- Joerg Schad


On Dec. 22, 2015, 10:27 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 22, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41661: Added documentation for API versioning.

[Anand Mazumdar]

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

(Updated Dec. 27, 2015, 9:08 p.m.)


Review request for mesos, Neil Conway and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change adds documentation on how Mesos does API versioning. It also has a 
section:
- On how API versioning and Release versioning are different.
- API compatibility/upgrade guarantees.
- Implementation Details.

Most of the information is taken from the design doc on Mesos HTTP API 
Versioning:
https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#


Diffs (updated)
-

  docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
  docs/versioning.md PRE-CREATION 

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


Testing
---

https://gist.github.com/hatred/1cc6db05d0ca51397886


Thanks,

Anand Mazumdar

Re: Review Request 41661: Added documentation for API versioning.

[Anand Mazumdar]

> On Dec. 27, 2015, 3:25 a.m., Jojy Varghese wrote:
> > docs/versioning.md, line 7
> > 
> >
> > ephasize 0.24.0 (in quotes). Same in other places.

I tried to be consistent to other docs and they did not have quotes.

e.g:
http://mesos.apache.org/documentation/latest/ssl/
http://mesos.apache.org/documentation/latest/framework-rate-limiting/
http://mesos.apache.org/documentation/latest/scheduler-http-api/


> On Dec. 27, 2015, 3:25 a.m., Jojy Varghese wrote:
> > docs/versioning.md, line 62
> > 
> >
> > This section could use some formatting. 
> > 
> > Also, would be useful to take the reader through a complete example 
> > covering a message's external and internal path.

Can you elaborate a bit more on what did you mean by:

```This section could use some formatting.```


- Anand


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


On Dec. 23, 2015, 2:09 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 23, 2015, 2:09 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41728: Removed docker provisioner port slave flags.

[Joerg Schad]

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


Basically all the same: Let us try to keep the documentation in sync :-).
One other question: Do we have a procedure for removing such flags? (i.e. mail 
to dev/user list, potential deprecation cycle?)?


src/slave/flags.cpp (line 124)


We should update the configuration documentation 
(https://github.com/apache/mesos/blob/master/docs/configuration.md)



src/slave/flags.cpp 


We should update the configuration documentation 
(https://github.com/apache/mesos/blob/master/docs/configuration.md)



src/slave/flags.cpp (line 144)


We should update the configuration documentation 
(https://github.com/apache/mesos/blob/master/docs/configuration.md)



src/slave/flags.cpp 


We should update the configuration documentation 
(https://github.com/apache/mesos/blob/master/docs/configuration.md)


- Joerg Schad


On Dec. 26, 2015, 8:28 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41728/
> ---
> 
> (Updated Dec. 26, 2015, 8:28 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed docker provisioner port slave flags.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> 798431c737691405a15989b1eb9ed5319ea0dacd 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/41728/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 41657: Added user facing documentation for containerizers.

[Jojy Varghese]

> On Dec. 27, 2015, 2:39 p.m., Joerg Schad wrote:
> > docs/containerizer.md, line 26
> > 
> >
> > What do you mean by "own native container technology"? Isn't that 
> > highly platform dependent?

Not sure what you mean. Here is my intent:

'Mesos containerizer' is Mesos's native container technology in terms of:
- Provides isolation between tasks
- Controls resources for tasks

It achieves these in Posix and linux platforms. Some platforms provide more 
control (linux) than others but the intent of the component is to  'contain' 
tasks.


- Jojy


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


On Dec. 22, 2015, 10:27 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 22, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41657: Added user facing documentation for containerizers.

[Jojy Varghese]

> On Dec. 28, 2015, 12:33 a.m., Qian Zhang wrote:
> > docs/containerizer.md, line 13
> > 
> >
> > Isolate task or executor or both? For task, I think it is up to 
> > executor for how to launch it.

Here the bullet points are for  what 'containers' are for (not containerizer).


- Jojy


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


On Dec. 22, 2015, 10:27 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 22, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41661: Added documentation for API versioning.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41661]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 27, 2015, 9:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41661/
> ---
> 
> (Updated Dec. 27, 2015, 9:08 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4192
> https://issues.apache.org/jira/browse/MESOS-4192
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds documentation on how Mesos does API versioning. It also has 
> a section:
> - On how API versioning and Release versioning are different.
> - API compatibility/upgrade guarantees.
> - Implementation Details.
> 
> Most of the information is taken from the design doc on Mesos HTTP API 
> Versioning:
> https://docs.google.com/document/d/1-iQjo6778H_fU_1Zi_Yk6szg8qj-wqYgVgnx7u3h6OU/edit#
> 
> 
> Diffs
> -
> 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
>   docs/versioning.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41661/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1cc6db05d0ca51397886
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41657: Added user facing documentation for containerizers.

[Qian Zhang]

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



docs/containerizer.md (line 13)


Isolate task or executor or both? For task, I think it is up to executor 
for how to launch it.


- Qian Zhang


On Dec. 23, 2015, 6:27 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41657/
> ---
> 
> (Updated Dec. 23, 2015, 6:27 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This documentation is intended for users trying to get an overview of
> containerizer technology. It also provides the criteria for selecting a
> particular type of containerizer.
> 
> 
> Diffs
> -
> 
>   docs/containerizer.md PRE-CREATION 
>   docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
> 
> Diff: https://reviews.apache.org/r/41657/diff/
> 
> 
> Testing
> ---
> 
> mesos website docker.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41731: Removed docker puller flag.

[Timothy Chen]

> On Dec. 27, 2015, 2:38 a.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp, line 79
> > 
> >
> > You might have to validate the flag for its format and maybe valid path.

Actually we assume it's file:// at this point, I can even add a CHECK() here.
We will know about valid path right when we do a look up in that folder.


- Timothy


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


On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> ---
> 
> (Updated Dec. 27, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed docker puller flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f71b572a32443a6715acb4d3541aec60e0437b30 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> dd17acf6a6e5c306029198dbb2a7e2d059f87f75 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/41731/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 41731: Removed docker puller flag.

[Timothy Chen]

> On Dec. 27, 2015, 5:42 a.m., Gilbert Song wrote:
> > src/slave/flags.cpp, lines 145-146
> > 
> >
> > Could we use pure directory only for local puller?
> > 
> > Instead of adding `file://` to the flag & strings::remove it, because 
> > some users may easily neglect a slash for this pattern.

We need to differentiate based on the flag's value to be local or registry as 
we consolidated a bunch of flags down to a single one now.


- Timothy


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


On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> ---
> 
> (Updated Dec. 27, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed docker puller flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f71b572a32443a6715acb4d3541aec60e0437b30 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> dd17acf6a6e5c306029198dbb2a7e2d059f87f75 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/41731/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 41731: Removed docker puller flag.

[Timothy Chen]

> On Dec. 27, 2015, 8:16 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 146
> > 
> >
> > Do I understand correctly that we change the default from "Pull from 
> > docker registry" to "pull from a local file location"? Wouldn't that impact 
> > users experimenting with Mesos? If this is not the case feel to drop.

The default has always been pulling from a local file location (docker_puller = 
local)


> On Dec. 27, 2015, 8:16 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 126
> > 
> >
> > When changing these flags we should also update the documentation: 
> > http://mesos.apache.org/documentation/latest/configuration/

Ah forgot about it, I'll add it.


- Timothy


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


On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> ---
> 
> (Updated Dec. 27, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed docker puller flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f71b572a32443a6715acb4d3541aec60e0437b30 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> dd17acf6a6e5c306029198dbb2a7e2d059f87f75 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/41731/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 41731: Removed docker puller flag.

[Joerg Schad]

> On Dec. 27, 2015, 8:16 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 146
> > 
> >
> > Do I understand correctly that we change the default from "Pull from 
> > docker registry" to "pull from a local file location"? Wouldn't that impact 
> > users experimenting with Mesos? If this is not the case feel to drop.
> 
> Timothy Chen wrote:
> The default has always been pulling from a local file location 
> (docker_puller = local)

THX!


- Joerg


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


On Dec. 27, 2015, 12:22 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41731/
> ---
> 
> (Updated Dec. 27, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed docker puller flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> f71b572a32443a6715acb4d3541aec60e0437b30 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> dd17acf6a6e5c306029198dbb2a7e2d059f87f75 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 7cdb15b9529eab82867b3470a016bb8ad09ff250 
>   src/slave/flags.hpp 1250786d1bdc8b312a1912a37ac8aac373dd5ec9 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> bb142f5ea99e8ea9b20a896f95ae37aa1d8d3f98 
> 
> Diff: https://reviews.apache.org/r/41731/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>