Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-29 Thread Jiang Yan Xu

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



include/mesos/mesos.proto (line 1213)
https://reviews.apache.org/r/34136/#comment147861

Hmm... I don't think this should be required. It's too inflexible and tasks 
likely will use name and labels.


- Jiang Yan Xu


On July 11, 2015, 9:47 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated July 11, 2015, 9:47 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-16 Thread Timothy Chen


 On July 14, 2015, 9:03 p.m., Jiang Yan Xu wrote:
  include/mesos/mesos.proto, lines 1211-1213
  https://reviews.apache.org/r/34136/diff/3/?file=1009139#file1009139line1211
 
  So I found the use of the field `id` inconsistent in the code.
  
  Sometimes `id` has the `sha512-` prefix and sometimes not.
  
  I think we should consistently refer to `id` using the definition in 
  the 
  [spec](https://github.com/appc/spec/blob/806b17c86ba5e5d595fca3f7ed339c8a22fb46c3/spec/aci.md#image-id),
   i.e., with the prefix.
  
  The fact that the ID is computed by the image creator using sha512 and 
  that the provisioner validates it using sha512 is merely an implementation 
  detail that is not a conern of higher level abstractions / APIs.
  
  So here I think in the comments we should not call it Image hash but 
  rather refer to the spec for its full definition. We can of course call out 
  the fact that it should have the sha512- perfix.
  
  What do you think?

Hi there,
I'm going to commit this for Ian and just saw your comment.
How about I reword the comment here to // The ID of the Image. Please refer to 
the Appc spec for its definition.?


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated July 12, 2015, 4:47 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-16 Thread Timothy Chen


 On July 14, 2015, 9:03 p.m., Jiang Yan Xu wrote:
  include/mesos/mesos.proto, lines 1211-1213
  https://reviews.apache.org/r/34136/diff/3/?file=1009139#file1009139line1211
 
  So I found the use of the field `id` inconsistent in the code.
  
  Sometimes `id` has the `sha512-` prefix and sometimes not.
  
  I think we should consistently refer to `id` using the definition in 
  the 
  [spec](https://github.com/appc/spec/blob/806b17c86ba5e5d595fca3f7ed339c8a22fb46c3/spec/aci.md#image-id),
   i.e., with the prefix.
  
  The fact that the ID is computed by the image creator using sha512 and 
  that the provisioner validates it using sha512 is merely an implementation 
  detail that is not a conern of higher level abstractions / APIs.
  
  So here I think in the comments we should not call it Image hash but 
  rather refer to the spec for its full definition. We can of course call out 
  the fact that it should have the sha512- perfix.
  
  What do you think?
 
 Timothy Chen wrote:
 Hi there,
 I'm going to commit this for Ian and just saw your comment.
 How about I reword the comment here to // The ID of the Image. Please 
 refer to the Appc spec for its definition.?
 
 Timothy Chen wrote:
 Actually I mis-read what you meant, how about:
 
  // The ID of the Image.
   // An image ID is canonically represented as a string prefixed by
   // the algorithm used and the hash output (e.g. sha512-a83...).
 
 Jiang Yan Xu wrote:
 Or we could just copy the definition here verbatim. :)
 
 https://github.com/appc/spec/blob/master/spec/types.md#image-id-type
 
 An image ID is a string of the format hash-value, where hash is the 
 hash algorithm used and value is the hex encoded string of the digest. 
 Currently the only permitted hash algorithm is sha512.
 
 Thanks, please commit it!

Thanks!


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated July 12, 2015, 4:47 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-16 Thread Jiang Yan Xu


 On July 14, 2015, 2:03 p.m., Jiang Yan Xu wrote:
  include/mesos/mesos.proto, lines 1211-1213
  https://reviews.apache.org/r/34136/diff/3/?file=1009139#file1009139line1211
 
  So I found the use of the field `id` inconsistent in the code.
  
  Sometimes `id` has the `sha512-` prefix and sometimes not.
  
  I think we should consistently refer to `id` using the definition in 
  the 
  [spec](https://github.com/appc/spec/blob/806b17c86ba5e5d595fca3f7ed339c8a22fb46c3/spec/aci.md#image-id),
   i.e., with the prefix.
  
  The fact that the ID is computed by the image creator using sha512 and 
  that the provisioner validates it using sha512 is merely an implementation 
  detail that is not a conern of higher level abstractions / APIs.
  
  So here I think in the comments we should not call it Image hash but 
  rather refer to the spec for its full definition. We can of course call out 
  the fact that it should have the sha512- perfix.
  
  What do you think?
 
 Timothy Chen wrote:
 Hi there,
 I'm going to commit this for Ian and just saw your comment.
 How about I reword the comment here to // The ID of the Image. Please 
 refer to the Appc spec for its definition.?
 
 Timothy Chen wrote:
 Actually I mis-read what you meant, how about:
 
  // The ID of the Image.
   // An image ID is canonically represented as a string prefixed by
   // the algorithm used and the hash output (e.g. sha512-a83...).

Or we could just copy the definition here verbatim. :)

https://github.com/appc/spec/blob/master/spec/types.md#image-id-type

An image ID is a string of the format hash-value, where hash is the hash 
algorithm used and value is the hex encoded string of the digest. Currently 
the only permitted hash algorithm is sha512.

Thanks, please commit it!


- Jiang Yan


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


On July 11, 2015, 9:47 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated July 11, 2015, 9:47 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-16 Thread Timothy Chen


 On July 14, 2015, 9:03 p.m., Jiang Yan Xu wrote:
  include/mesos/mesos.proto, lines 1211-1213
  https://reviews.apache.org/r/34136/diff/3/?file=1009139#file1009139line1211
 
  So I found the use of the field `id` inconsistent in the code.
  
  Sometimes `id` has the `sha512-` prefix and sometimes not.
  
  I think we should consistently refer to `id` using the definition in 
  the 
  [spec](https://github.com/appc/spec/blob/806b17c86ba5e5d595fca3f7ed339c8a22fb46c3/spec/aci.md#image-id),
   i.e., with the prefix.
  
  The fact that the ID is computed by the image creator using sha512 and 
  that the provisioner validates it using sha512 is merely an implementation 
  detail that is not a conern of higher level abstractions / APIs.
  
  So here I think in the comments we should not call it Image hash but 
  rather refer to the spec for its full definition. We can of course call out 
  the fact that it should have the sha512- perfix.
  
  What do you think?
 
 Timothy Chen wrote:
 Hi there,
 I'm going to commit this for Ian and just saw your comment.
 How about I reword the comment here to // The ID of the Image. Please 
 refer to the Appc spec for its definition.?

Actually I mis-read what you meant, how about:

 // The ID of the Image.
  // An image ID is canonically represented as a string prefixed by
  // the algorithm used and the hash output (e.g. sha512-a83...).


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated July 12, 2015, 4:47 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-14 Thread Jiang Yan Xu

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


Sorry I didn't notice it in my original review but I traced the issue back from 
future reviews.


include/mesos/mesos.proto (lines 1211 - 1213)
https://reviews.apache.org/r/34136/#comment145213

So I found the use of the field `id` inconsistent in the code.

Sometimes `id` has the `sha512-` prefix and sometimes not.

I think we should consistently refer to `id` using the definition in the 
[spec](https://github.com/appc/spec/blob/806b17c86ba5e5d595fca3f7ed339c8a22fb46c3/spec/aci.md#image-id),
 i.e., with the prefix.

The fact that the ID is computed by the image creator using sha512 and that 
the provisioner validates it using sha512 is merely an implementation detail 
that is not a conern of higher level abstractions / APIs.

So here I think in the comments we should not call it Image hash but 
rather refer to the spec for its full definition. We can of course call out the 
fact that it should have the sha512- perfix.

What do you think?


- Jiang Yan Xu


On July 11, 2015, 9:47 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated July 11, 2015, 9:47 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-14 Thread Jiang Yan Xu


 On June 26, 2015, 2:57 p.m., Jiang Yan Xu wrote:
  include/mesos/mesos.proto, lines 1212-1214
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1212
 
  Is it the intention that Image type is **defined** outside MesosInfo 
  because DockerInfo can later reference it?
  
  Otherwise it feels more natual to define Image within MesosInfo.
 
 Vinod Kone wrote:
 +1 to put it inside MesosInfo.
 
 Ian Downes wrote:
 I wanted it to be outside MesosInfo so it could be used by other 
 container types, i.e., I don't see it as specific to MesosInfo, e.g., we 
 could also support an AppcInfo which contained an Image::APPC.

OK SGTM.


- Jiang Yan


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


On July 11, 2015, 9:47 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated July 11, 2015, 9:47 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-11 Thread Ian Downes


 On June 25, 2015, 3:32 p.m., Paul Brett wrote:
  include/mesos/mesos.proto, line 1221
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1221
 
  Mesos info is optional, but if present can optionally contain an image? 
   So what does a mesos present with a MesosInfo message with no image mean?
 
 Vinod Kone wrote:
 yea. why optional?
 
 Timothy Chen wrote:
 I think image can be optional which bypasses the image provisioning. 
 MesosInfo potentially will contain other information like what DockerInfo 
 does that is MesosContainerizer specific, so I think this API makes sense to 
 me.

Yep, Tim's comments reflect what I'm thinking.


 On June 25, 2015, 3:32 p.m., Paul Brett wrote:
  include/mesos/mesos.proto, line 1202
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1202
 
  Not really an id, more of a signature for the image.  Would we be 
  better making this optional string sha512hash to allow for the possibility 
  that a different signature might be used in the future?
 
 Vinod Kone wrote:
 i'm assuming this is because appc calls it an image id.

Correct, this reflects naming in the appc spec.


- Ian


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


On June 22, 2015, 9:42 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 9:42 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-11 Thread Ian Downes


 On June 26, 2015, 2:57 p.m., Jiang Yan Xu wrote:
  include/mesos/mesos.proto, lines 1212-1214
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1212
 
  Is it the intention that Image type is **defined** outside MesosInfo 
  because DockerInfo can later reference it?
  
  Otherwise it feels more natual to define Image within MesosInfo.
 
 Vinod Kone wrote:
 +1 to put it inside MesosInfo.

I wanted it to be outside MesosInfo so it could be used by other container 
types, i.e., I don't see it as specific to MesosInfo, e.g., we could also 
support an AppcInfo which contained an Image::APPC.


- Ian


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


On June 22, 2015, 9:42 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 9:42 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-08 Thread Vinod Kone


 On June 26, 2015, 9:57 p.m., Jiang Yan Xu wrote:
  include/mesos/mesos.proto, lines 1212-1214
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1212
 
  Is it the intention that Image type is **defined** outside MesosInfo 
  because DockerInfo can later reference it?
  
  Otherwise it feels more natual to define Image within MesosInfo.

+1 to put it inside MesosInfo.


- Vinod


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


On June 22, 2015, 4:42 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 4:42 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-08 Thread Timothy Chen


 On June 25, 2015, 10:32 p.m., Paul Brett wrote:
  include/mesos/mesos.proto, line 1221
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1221
 
  Mesos info is optional, but if present can optionally contain an image? 
   So what does a mesos present with a MesosInfo message with no image mean?
 
 Vinod Kone wrote:
 yea. why optional?

I think image can be optional which bypasses the image provisioning. MesosInfo 
potentially will contain other information like what DockerInfo does that is 
MesosContainerizer specific, so I think this API makes sense to me.


- Timothy


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


On June 22, 2015, 4:42 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 4:42 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-08 Thread Vinod Kone

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

Ship it!


LGTM.


include/mesos/mesos.proto (line 1220)
https://reviews.apache.org/r/34136/#comment144200

Add a comment that only one of these should be set.


- Vinod Kone


On June 22, 2015, 4:42 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 4:42 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-08 Thread Vinod Kone


 On June 25, 2015, 10:32 p.m., Paul Brett wrote:
  include/mesos/mesos.proto, line 1221
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1221
 
  Mesos info is optional, but if present can optionally contain an image? 
   So what does a mesos present with a MesosInfo message with no image mean?

yea. why optional?


 On June 25, 2015, 10:32 p.m., Paul Brett wrote:
  include/mesos/mesos.proto, line 1202
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1202
 
  Not really an id, more of a signature for the image.  Would we be 
  better making this optional string sha512hash to allow for the possibility 
  that a different signature might be used in the future?

i'm assuming this is because appc calls it an image id.


- Vinod


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


On June 22, 2015, 4:42 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 4:42 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-01 Thread Lily Chen

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



include/mesos/mesos.proto (line 1209)
https://reviews.apache.org/r/34136/#comment143158

Same issue as when MesosInfo is not present. What happens when there is no 
Appc message present in Image?


- Lily Chen


On June 22, 2015, 4:42 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 4:42 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-06-26 Thread Jiang Yan Xu

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



include/mesos/mesos.proto (lines 1212 - 1214)
https://reviews.apache.org/r/34136/#comment142204

Is it the intention that Image type is **defined** outside MesosInfo 
because DockerInfo can later reference it?

Otherwise it feels more natual to define Image within MesosInfo.


- Jiang Yan Xu


On June 22, 2015, 9:42 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 9:42 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-06-25 Thread Paul Brett

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



include/mesos/mesos.proto (line 1196)
https://reviews.apache.org/r/34136/#comment142070

I assume that much of this is prescribed by the AppC specification - can we 
reference it?



include/mesos/mesos.proto (line 1202)
https://reviews.apache.org/r/34136/#comment142067

Not really an id, more of a signature for the image.  Would we be better 
making this optional string sha512hash to allow for the possibility that a 
different signature might be used in the future?



include/mesos/mesos.proto (line 1221)
https://reviews.apache.org/r/34136/#comment142073

Mesos info is optional, but if present can optionally contain an image?  So 
what does a mesos present with a MesosInfo message with no image mean?


- Paul Brett


On June 22, 2015, 4:42 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 4:42 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-06-24 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On June 22, 2015, 4:42 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 4:42 p.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-05-14 Thread Timothy Chen

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



include/mesos/mesos.proto
https://reviews.apache.org/r/34136/#comment134887

BenH left a similar comment earlier last time when we looked at the new 
changes, and basically I think instead of calling it just ContainerImage we 
should make sure it's Appc specific, in case there are other formats we want to 
support in the future and now we have to deprecate and reintroduce new ones 
like we did the in past.


- Timothy Chen


On May 13, 2015, 12:47 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated May 13, 2015, 12:47 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto db4fc8c001dd68bc3b9ca83650170c4f26db18c7 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes