Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-09-06 Thread Jie Yu 

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




include/mesos/mesos.proto
Lines 1044 (patched)


Also, I'd suggest we put `id` into `source`. Given that the CSI spec 
change, we'll use string for `id` and `Labels` for metadata:

```
message Source {
  optional string id;
  optional Labels metadata;
}
```


- Jie Yu


On May 19, 2017, 9:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated May 19, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e138e2aedb1369acdde82facf9dfea9780411154 
>   include/mesos/v1/mesos.proto ab617bfd2bc46e44f7d71cefecf5eac3630d22b3 
>   src/common/resources.cpp 1d07f1a049ba3bb3f5d78f05031f33ba97e07e8c 
>   src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
>   src/tests/resources_tests.cpp 5dcbce2dd4944194b59790551929d6d75b805ba3 
>   src/v1/resources.cpp 1bb5d0741c9f9ea59f34e92159a335661019444d 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/4/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-09-02 Thread Jie Yu 

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



This might need to be adjusted based on this:
https://github.com/container-storage-interface/spec/pull/97

so `id` should be string, and `metadata` is labels given the above PR goes 
through

- Jie Yu


On May 19, 2017, 9:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated May 19, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e138e2aedb1369acdde82facf9dfea9780411154 
>   include/mesos/v1/mesos.proto ab617bfd2bc46e44f7d71cefecf5eac3630d22b3 
>   src/common/resources.cpp 1d07f1a049ba3bb3f5d78f05031f33ba97e07e8c 
>   src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
>   src/tests/resources_tests.cpp 5dcbce2dd4944194b59790551929d6d75b805ba3 
>   src/v1/resources.cpp 1bb5d0741c9f9ea59f34e92159a335661019444d 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/4/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-08-28 Thread Jie Yu 

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




src/common/resources.cpp
Line 187 (original), 187 (patched)


We also need to update `addable` and `subtractable` similar to what we did 
for persistent volumes.

Depending on the "disk" source type, the semantics is different:
1. RAW w/ id:
   - Cannot be split into smaller pieces
   - Cannot be merged even if they are equal
2. RAW w/o id:
   - Can be split into smaller pieces
   - Can be merged if RAW parts equal
3. PATH w/ id:
   - Can be split into smaller pieces
   - Can be merged if PATH parts equal
4. PATH w/o id:
   - Same as PATH w/ id
5. MOUNT w/ id:
   - Cannot be split into smaller pieces
   - Cannot be merged even if they are equal
6. MOUNT w/o id:
   - Same as MOUNT w/ id


- Jie Yu


On May 19, 2017, 9:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated May 19, 2017, 9:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e138e2aedb1369acdde82facf9dfea9780411154 
>   include/mesos/v1/mesos.proto ab617bfd2bc46e44f7d71cefecf5eac3630d22b3 
>   src/common/resources.cpp 1d07f1a049ba3bb3f5d78f05031f33ba97e07e8c 
>   src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
>   src/tests/resources_tests.cpp 5dcbce2dd4944194b59790551929d6d75b805ba3 
>   src/v1/resources.cpp 1bb5d0741c9f9ea59f34e92159a335661019444d 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/4/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-05-19 Thread Benjamin Bannier 

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

(Updated May 19, 2017, 11:16 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed issue raised by Qian.


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


Repository: mesos


Description
---

Ids will allow to create distinguishable resources, e.g., of RAW or
BLOCK type.


Diffs (updated)
-

  include/mesos/mesos.proto e138e2aedb1369acdde82facf9dfea9780411154 
  include/mesos/v1/mesos.proto ab617bfd2bc46e44f7d71cefecf5eac3630d22b3 
  src/common/resources.cpp 1d07f1a049ba3bb3f5d78f05031f33ba97e07e8c 
  src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
  src/tests/resources_tests.cpp 5dcbce2dd4944194b59790551929d6d75b805ba3 
  src/v1/resources.cpp 1bb5d0741c9f9ea59f34e92159a335661019444d 


Diff: https://reviews.apache.org/r/58048/diff/4/

Changes: https://reviews.apache.org/r/58048/diff/3-4/


Testing
---

make check (OS X, Fedora25)


Thanks,

Benjamin Bannier

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-05-19 Thread Benjamin Bannier 


> On May 17, 2017, 5:12 p.m., Qian Zhang wrote:
> > src/tests/resources_tests.cpp
> > Lines 2190 (patched)
> > 
> >
> > Unused variable?

Thanks, also fixed the other sites.


- Benjamin


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


On May 19, 2017, 11:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated May 19, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e138e2aedb1369acdde82facf9dfea9780411154 
>   include/mesos/v1/mesos.proto ab617bfd2bc46e44f7d71cefecf5eac3630d22b3 
>   src/common/resources.cpp 1d07f1a049ba3bb3f5d78f05031f33ba97e07e8c 
>   src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
>   src/tests/resources_tests.cpp 5dcbce2dd4944194b59790551929d6d75b805ba3 
>   src/v1/resources.cpp 1bb5d0741c9f9ea59f34e92159a335661019444d 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/4/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-05-19 Thread Benjamin Bannier 


> On April 17, 2017, 11:49 a.m., Jie Yu wrote:
> > src/tests/resources_tests.cpp
> > Lines 2255 (patched)
> > 
> >
> > instead of relying on 'count', let's use `size()` instead.
> 
> Qian Zhang wrote:
> I think here the intention is to count how many `disks` are in `(r1 + 
> r2)`, so `count()` should be the right method to call.

Yes, exactly. Note that we could use `size` and explicit value checks here, but 
this would introduce dependencies to the exact disk values which in my opinion 
makes the code harder to understand.


- Benjamin


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


On May 19, 2017, 11:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated May 19, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e138e2aedb1369acdde82facf9dfea9780411154 
>   include/mesos/v1/mesos.proto ab617bfd2bc46e44f7d71cefecf5eac3630d22b3 
>   src/common/resources.cpp 1d07f1a049ba3bb3f5d78f05031f33ba97e07e8c 
>   src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
>   src/tests/resources_tests.cpp 5dcbce2dd4944194b59790551929d6d75b805ba3 
>   src/v1/resources.cpp 1bb5d0741c9f9ea59f34e92159a335661019444d 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/4/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-05-17 Thread Qian Zhang 


> On April 17, 2017, 5:49 p.m., Jie Yu wrote:
> > src/tests/resources_tests.cpp
> > Lines 2255 (patched)
> > 
> >
> > instead of relying on 'count', let's use `size()` instead.

I think here the intention is to count how many `disks` are in `(r1 + r2)`, so 
`count()` should be the right method to call.


- Qian


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


On April 5, 2017, 9:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated April 5, 2017, 9:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 228623155c7f68c0f24d173aacbc6eb734f1382f 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/mesos.hpp fe897c184829910addec95ace174546092a9e2b2 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/3/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-05-17 Thread Qian Zhang 

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




src/tests/resources_tests.cpp
Lines 2190 (patched)


Unused variable?


- Qian Zhang


On April 5, 2017, 9:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated April 5, 2017, 9:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 228623155c7f68c0f24d173aacbc6eb734f1382f 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/mesos.hpp fe897c184829910addec95ace174546092a9e2b2 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/3/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-04-17 Thread Jie Yu 

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


Fix it, then Ship it!





src/tests/resources_tests.cpp
Lines 2255 (patched)


instead of relying on 'count', let's use `size()` instead.


- Jie Yu


On April 5, 2017, 1:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated April 5, 2017, 1:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 228623155c7f68c0f24d173aacbc6eb734f1382f 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/mesos.hpp fe897c184829910addec95ace174546092a9e2b2 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/3/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-04-05 Thread Benjamin Bannier 

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

(Updated April 5, 2017, 3:56 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Rewrapped a line, via Jan.


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


Repository: mesos


Description
---

Ids will allow to create distinguishable resources, e.g., of RAW or
BLOCK type.


Diffs (updated)
-

  include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
  include/mesos/v1/mesos.proto 228623155c7f68c0f24d173aacbc6eb734f1382f 
  src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
  src/tests/mesos.hpp fe897c184829910addec95ace174546092a9e2b2 
  src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
  src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 


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

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


Testing
---

make check (OS X, Fedora25)


Thanks,

Benjamin Bannier

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-04-03 Thread Jan Schlicht 

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


Fix it, then Ship it!





src/tests/mesos.hpp
Line 1356 (original), 1374 (patched)


Please add a line break after `<` to remove "jaggedness", as it's done, 
e.g., in `createTaskGroupInfo` above.


- Jan Schlicht


On March 31, 2017, 11:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58048/
> ---
> 
> (Updated March 31, 2017, 11:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ids will allow to create distinguishable resources, e.g., of RAW or
> BLOCK type.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/mesos.hpp fe897c184829910addec95ace174546092a9e2b2 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/58048/diff/2/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58048: Added id to Resource.DiskInfo.

2017-03-31 Thread Benjamin Bannier 

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

(Updated March 31, 2017, 11:38 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Added some tests.


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


Repository: mesos


Description
---

Ids will allow to create distinguishable resources, e.g., of RAW or
BLOCK type.


Diffs (updated)
-

  include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
  include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
  src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
  src/tests/mesos.hpp fe897c184829910addec95ace174546092a9e2b2 
  src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
  src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 


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

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


Testing
---

make check (OS X, Fedora25)


Thanks,

Benjamin Bannier