Review Request 38447: Implemented a TODO to clean up host mounts irrelevant to the container in the container's mount namespace.

2015-09-16 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38447/ --- Review request for mesos and Jie Yu. Bugs: MESOS-3433 https://issues.apache

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Guangya Liu
> On Sept. 16, 2015, 1:43 a.m., Guangya Liu wrote: > > > > Timothy Chen wrote: > it looks like the structs are ordered by response code, so just moving > this to the right place assuming so Got it, this can make the code clear. Thanks. - Guangya

Re: Review Request 38444: Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.

2015-09-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38444/#review99353 --- Patch looks great! Reviews applied: [38444] All tests passed. - M

Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-16 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38443/#review99351 --- src/slave/containerizer/provisioner/docker/registry_client.cpp (lin

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Timothy Chen
> On Sept. 16, 2015, 10:42 p.m., Ben Mahler wrote: > > Looks like you're missing the change to decoder. On that note, what happens > > when we receive a response with a code that we haven't added to our enum? > > Timothy Chen wrote: > You're right I missed the decoding part! Let me populate

Re: Review Request 38444: Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.

2015-09-16 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38444/#review99348 --- Ship it! src/slave/containerizer/isolators/filesystem/linux.cpp (l

Re: Review Request 38378: Minor cleanup in perf_tests.cpp.

2015-09-16 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38378/#review99347 --- Ship it! Ship It! - Guangya Liu On 九月 14, 2015, 11:08 p.m., Ben

Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38443/#review99346 --- Patch looks great! Reviews applied: [38443] All tests passed. - M

Re: Review Request 37505: Fix broken health check in docker executor.

2015-09-16 Thread haosdent huang
> On Sept. 16, 2015, 8:39 p.m., Timothy Chen wrote: > > Ship It! Thank you. Now let me upload a backport patches for 0.24 and 0.23 - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3

Re: Review Request 38444: Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.

2015-09-16 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38444/#review99343 --- Ship it! Ship It! - Timothy Chen On Sept. 17, 2015, 12:52 a.m.,

Re: Review Request 38003: MESOS-3351 (duplicated slave id in master after master failover)

2015-09-16 Thread Klaus Ma
> On Sept. 15, 2015, 10:32 p.m., Vinod Kone wrote: > > src/tests/master_tests.cpp, line 3637 > > > > > > Does this test reliably fail (i.e., every time) without the code change > > in master.cpp? Nop; the repro ra

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38253/#review99340 --- include/mesos/mesos.proto (line 820)

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-16 Thread Niklas Nielsen
> On Sept. 14, 2015, 11:07 a.m., Niklas Nielsen wrote: > > src/tests/oversubscription_tests.cpp, line 859 > > > > > > Mind adding a TODO about doing a full blown object comparison? > > Klaus Ma wrote: > Do you m

Review Request 38444: Only unmount sandbox if the mount exists in LinuxFilesystemIsolator.

2015-09-16 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38444/ --- Review request for mesos, Timothy Chen and Jiang Yan Xu. Repository: mesos De

Review Request 38443: Added layerid information to ManifestResponse

2015-09-16 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38443/ --- Review request for mesos and Timothy Chen. Repository: mesos Description

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38253/#review99338 --- Patch looks great! Reviews applied: [38253] All tests passed. - M

Re: Review Request 37931: Update indent for slave.cpp

2015-09-16 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37931/#review99335 --- Ship it! Ship It! - Ben Mahler On Sept. 15, 2015, 11:25 a.m., Gu

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

2015-09-16 Thread Klaus Ma
> On Sept. 14, 2015, 6:07 p.m., Niklas Nielsen wrote: > > include/mesos/slave/oversubscription.proto, lines 46-47 > > > > > > We need to add a comment about the semantics of executor_id vs > > container_id. For examp

Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

2015-09-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38368/#review99334 --- Ship it! Ship It! - Niklas Nielsen On Sept. 16, 2015, 3 p.m., Ka

Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Ben Mahler
> On Sept. 16, 2015, 11:04 p.m., Ben Mahler wrote: > > Whoops, it would be better for all of these to be > > process::MAX_REAP_INTERVAL() since we'd like to move away from using entire > > the entire process namespace in the .cpp files. > > Guangya Liu wrote: > OK, I will try to file anoth

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Ben Mahler
> On Sept. 16, 2015, 10:42 p.m., Ben Mahler wrote: > > Looks like you're missing the change to decoder. On that note, what happens > > when we receive a response with a code that we haven't added to our enum? > > Timothy Chen wrote: > You're right I missed the decoding part! Let me populate

Re: Review Request 38168: libprocess: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38168/#review99331 --- Ship it! Ship It! - Ben Mahler On Sept. 16, 2015, 4:07 p.m., Gua

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Timothy Chen
> On Sept. 16, 2015, 10:42 p.m., Ben Mahler wrote: > > Looks like you're missing the change to decoder. On that note, what happens > > when we receive a response with a code that we haven't added to our enum? You're right I missed the decoding part! Let me populate all the codes in the enum an

Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu
> On 九月 16, 2015, 11:04 p.m., Ben Mahler wrote: > > Whoops, it would be better for all of these to be > > process::MAX_REAP_INTERVAL() since we'd like to move away from using entire > > the entire process namespace in the .cpp files. OK, I will try to file another bug to resolve this. Thanks.

Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38046/#review99328 --- Whoops, it would be better for all of these to be process::MAX_REAP_

Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38046/#review99327 --- Ship it! Ship It! - Ben Mahler On Sept. 16, 2015, 4:07 p.m., Gua

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38370/#review99326 --- Patch looks great! Reviews applied: [38363, 38364, 38365, 38366, 38

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38416/#review99324 --- Looks like you're missing the change to decoder. On that note, what

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38416/#review99319 --- Ship it! This improvement brings convenience. - Gilbert Song On

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-16 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38367/ --- (Updated Sept. 16, 2015, 6 p.m.) Review request for mesos, Connor Doyle, Jie Yu

Re: Review Request 38365: Updated Isolator::prepare to return list of required namespaces.

2015-09-16 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38365/ --- (Updated Sept. 16, 2015, 6 p.m.) Review request for mesos, Connor Doyle, Jie Yu

Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

2015-09-16 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38368/ --- (Updated Sept. 16, 2015, 6 p.m.) Review request for mesos, Connor Doyle, Jie Yu

Re: Review Request 38364: Minor refactor for MesosContainerizer.

2015-09-16 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38364/ --- (Updated Sept. 16, 2015, 6 p.m.) Review request for mesos, Connor Doyle, Jie Yu

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-16 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38366/ --- (Updated Sept. 16, 2015, 6 p.m.) Review request for mesos, Connor Doyle, Jie Yu

Re: Review Request 38363: Refactored container Launcher to accept namespaces dynamically.

2015-09-16 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38363/ --- (Updated Sept. 16, 2015, 6 p.m.) Review request for mesos, Connor Doyle, Jie Yu

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38370/ --- (Updated Sept. 16, 2015, 6 p.m.) Review request for mesos, Connor Doyle, Jie Yu

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu
> On Sept. 16, 2015, 9:39 p.m., Jiang Yan Xu wrote: > > src/tests/containerizer/provisioner_appc_tests.cpp, lines 290-295 > > > > > > I was using this to test these paths::XYZ methods: Code being tested is > > using

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38417/#review99279 --- Ship it! src/slave/containerizer/provisioner/appc/store.cpp (line

Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

2015-09-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38368/#review99306 --- src/examples/test_hook_module.cpp (line 203)

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38367/#review99303 --- To Vinod's point; can we use JSON::protobuf() and override the field

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-16 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review99305 --- Read through all the codes. A discussion is needed to determine whet

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38416/#review99304 --- Ship it! Ship It! - Joseph Wu On Sept. 15, 2015, 6:03 p.m., Timo

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-16 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38367/#review99301 --- Ship it! include/mesos/mesos.proto (lines 1165 - 1167)

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

2015-09-16 Thread Vinod Kone
> On Sept. 14, 2015, 10:19 p.m., Vinod Kone wrote: > > src/common/http.cpp, lines 153-194 > > > > > > Why custom models? Can we not use the protobuf to json converters? > > Kapil Arya wrote: > This is mostly due

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38370/#review99297 --- Bad patch! Reviews applied: [38363, 38364, 38365] Failed command:

Re: Review Request 37826: Introduced conversion of JSON arrays to repeated protobufs.

2015-09-16 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37826/#review99296 --- Ship it! Ship It! - Gilbert Song On Sept. 10, 2015, 6:31 p.m., A

Re: Review Request 37505: Fix broken health check in docker executor.

2015-09-16 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37505/#review99294 --- Ship it! Ship It! - Timothy Chen On Sept. 14, 2015, 12:59 p.m.,

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-16 Thread Gilbert Song
> On Sept. 15, 2015, 12:17 a.m., Jie Yu wrote: > > src/slave/containerizer/provisioners/docker/provisioner.cpp, lines 376-379 > > > > > > The logic here also changes in AppcProvisioner. Please make sure it's > > in

Re: Review Request 38137: Added Docker provisioner and local store

2015-09-16 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review99048 --- src/slave/containerizer/provisioners/docker/metadata_manager.hpp (l

Re: Review Request 38011: Maintenance Primitives: Use the parse> helper instead of a plural MachineID protobuf.

2015-09-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38011/#review99292 --- Patch looks great! Reviews applied: [38011] All tests passed. - M

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38370/ --- (Updated Sept. 16, 2015, 3:52 p.m.) Review request for mesos, Connor Doyle, Jie

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Ben Mahler
> On Sept. 16, 2015, 6:01 p.m., Ben Mahler wrote: > > Have you surveyed how other libraires handle this? For example, the scala > > play framework seems to use BigDecimal (128 bit floating point) for all > > numbers, which can represent all 64 bit signed / unsigned integral values > > without

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38370/#review99289 --- Ship it! Ship It! - Niklas Nielsen On Sept. 16, 2015, 12:52 p.m.

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Kapil Arya
> On Sept. 16, 2015, 3:44 p.m., Niklas Nielsen wrote: > > src/docker/executor.cpp, lines 162-164 > > > > > > Was "Docker.NetworkSettings.IPAddress" in 0.24.0? If so, don't we want > > to deprecate this over a releas

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38370/#review99284 --- src/docker/executor.cpp (lines 162 - 164)

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

2015-09-16 Thread Niklas Nielsen
> On Sept. 15, 2015, 6:57 p.m., Mesos ReviewBot wrote: > > Bad patch! > > > > Reviews applied: [38363, 38364, 38365] > > > > Failed command: ./support/apply-review.sh -n -r 38365 > > > > Error: > > 2015-09-16 01:57:34 URL:https://reviews.apache.org/r/38365/diff/raw/ > > [16233/16233] -> "383

Re: Review Request 38366: Added helper to model Labels message for state.json.

2015-09-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38366/#review99282 --- Ship it! Ship It! - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m.,

Re: Review Request 38011: Maintenance Primitives: Use the parse> helper instead of a plural MachineID protobuf.

2015-09-16 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38011/ --- (Updated Sept. 16, 2015, 12:38 p.m.) Review request for mesos, Alexander Ruklet

Re: Review Request 38363: Refactored container Launcher to accept namespaces dynamically.

2015-09-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38363/#review99281 --- Ship it! Ship It! - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m.,

Re: Review Request 38011: Maintenance Primitives: Use the parse> helper instead of a plural MachineID protobuf.

2015-09-16 Thread Joseph Wu
> On Sept. 15, 2015, 11:58 p.m., Guangya Liu wrote: > > src/common/protobuf_utils.hpp, line 115 > > > > > > This should also be removed. Thanks for spotting this :) - Joseph -

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Joseph Wu
> On Sept. 16, 2015, 11:01 a.m., Ben Mahler wrote: > > Have you surveyed how other libraires handle this? For example, the scala > > play framework seems to use BigDecimal (128 bit floating point) for all > > numbers, which can represent all 64 bit signed / unsigned integral values > > without

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu
> On Sept. 16, 2015, 10:54 a.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --i

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Vinod Kone
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu
> On Sept. 16, 2015, 10:54 a.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --i

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Ben Mahler
> On Sept. 16, 2015, 6:01 p.m., Ben Mahler wrote: > > Have you surveyed how other libraires handle this? For example, the scala > > play framework seems to use BigDecimal (128 bit floating point) for all > > numbers, which can represent all 64 bit signed / unsigned integral values > > without

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Joris Van Remoortere
> On Sept. 16, 2015, 6:01 p.m., Ben Mahler wrote: > > Have you surveyed how other libraires handle this? For example, the scala > > play framework seems to use BigDecimal (128 bit floating point) for all > > numbers, which can represent all 64 bit signed / unsigned integral values > > without

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` > > Vinod Kone wrote: > one singular one plural? can we name the latter --im

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Vinod Kone
> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote: > > Chatted with Viond about the flag naming. We prefer the following: > > > > ``` > > --image_providers=APPC,DOCKER > > --image_provisioner=bind,copy,overlayfs > > ``` one singular one plural? can we name the latter --image_provisioners? - Vinod

Re: Review Request 38031: [3/5] Integer Precision for JSON <-> Protobuf conversions.

2015-09-16 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38031/#review99254 --- Have you surveyed how other libraires handle this? For example, the

Re: Review Request 37505: Fix broken health check in docker executor.

2015-09-16 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37505/#review99251 --- By the way your patch fails mesos style check. Did you run the mesos

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38417/#review99250 --- Chatted with Viond about the flag naming. We prefer the following:

Re: Review Request 37505: Fix broken health check in docker executor.

2015-09-16 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37505/#review99246 --- Overall looks good to me, some minor comments around style and wordi

Re: Review Request 38168: libprocess: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu
> On 九月 16, 2015, 3:09 p.m., Alexander Rukletsov wrote: > > Last minor nit: we tend to use past tense in summary and prepend the commit > > message with "libprocess" for libprocess changes. Also, not a trailing > > period. That's how it may be rewritten: > > "libprocess: Replaced hard-coded rea

Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu
> On 九月 16, 2015, 3:12 p.m., Alexander Rukletsov wrote: > > Looks good! Similar to my comment on https://reviews.apache.org/r/38168/, > > please adjust the summary to something like "mesos: Replaced hard-coded > > reap interval with a constant." Thanks Alex, updated. - Guangya

Re: Review Request 38168: libprocess: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38168/ --- (Updated 九月 16, 2015, 4:07 p.m.) Review request for mesos, Alexander Rukletsov

Re: Review Request 38046: mesos: Replaced hard-coded reap interval with a constant.

2015-09-16 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38046/ --- (Updated 九月 16, 2015, 4:07 p.m.) Review request for mesos, Alexander Rukletsov

Re: Review Request 38056: Fix webui task informations not update bug.

2015-09-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38056/#review99229 --- Patch looks great! Reviews applied: [38056] All tests passed. - M

Re: Review Request 38046: Replace hard-coded reap interval with a constant

2015-09-16 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38046/#review99225 --- Ship it! Looks good! Similar to my comment on https://reviews.apach

Re: Review Request 38168: Replace hard-coded reap interval with a constant for 3rdparty

2015-09-16 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38168/#review99224 --- Ship it! Last minor nit: we tend to use past tense in summary and p

Re: Review Request 38056: Fix webui task informations not update bug.

2015-09-16 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38056/ --- (Updated Sept. 16, 2015, 3:03 p.m.) Review request for mesos, Adam B, Joerg Sch

Re: Review Request 38056: Fix webui task informations not update bug.

2015-09-16 Thread haosdent huang
> On Sept. 16, 2015, 10:41 a.m., Philip Norman wrote: > > src/webui/master/static/js/controllers.js, line 224 > > > > > > These metrics are not available on master, only directly from the slave > > nodes. > >

Re: Review Request 38056: Fix webui task informations not update bug.

2015-09-16 Thread Philip Norman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38056/#review99207 --- src/webui/master/static/js/controllers.js (line 217)

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

2015-09-16 Thread Zhiwei Chen
> On Sept. 16, 2015, 12:53 a.m., Joseph Wu wrote: > > include/mesos/authorizer/authorizer.proto, lines 78-79 > > > > > > Consider renaming to `machine_ids`. > > > > You should also consider a string represent

Re: Review Request 38278: Updating unversioned Executor protobuf

2015-09-16 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38278/#review99204 --- Ship it! Ship It! - Guangya Liu On 九月 15, 2015, 10:24 p.m., Isab

Re: Review Request 38124: Add V1 Support for QuiesceOffers

2015-09-16 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38124/#review99197 --- Patch looks great! Reviews applied: [37532, 37873, 38119, 38120, 38

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-16 Thread Isabel Jimenez
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38416/#review99193 --- Ship it! Ship It! - Isabel Jimenez On Sept. 16, 2015, 1:03 a.m.,