Re: Review Request 38443: Added layerid information to ManifestResponse

2015-10-13 Thread Ben Mahler


> On Oct. 13, 2015, 12:27 a.m., Timothy Chen wrote:
> > Ship It!

Tim if this looks good to you can you commit? I'd like to work through the 
cleanup chain that follows this :)


- Ben


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


On Oct. 9, 2015, 6:34 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Oct. 9, 2015, 6:34 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> fdb68b675582f603ffb3e96f31c1c9405e238567 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-10-12 Thread Timothy Chen

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

Ship it!


Ship It!

- Timothy Chen


On Oct. 9, 2015, 6:34 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Oct. 9, 2015, 6:34 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> fdb68b675582f603ffb3e96f31c1c9405e238567 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-10-09 Thread Jojy Varghese

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

(Updated Oct. 9, 2015, 6:34 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

Split change into small patches


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-10-05 Thread Jojy Varghese

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

(Updated Oct. 6, 2015, 2:31 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

reordered patches


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
c2040b48ea43fdb29766994c244273d3fa9ee3cd 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-28 Thread Timothy Chen

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



src/slave/containerizer/provisioner/docker/registry_client.cpp (line 79)


tokenMgr -> tokenManager, I can fix myself



src/slave/containerizer/provisioner/docker/registry_client.cpp (line 505)


I think we need to say we failed because it's not the expected Object type. 
Same here and other placess you have the check

Also you use index: in other places, and just index here. Let's be 
consistent and use index:


- Timothy Chen


On Sept. 25, 2015, 6:33 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 25, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-28 Thread Jojy Varghese

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

(Updated Sept. 28, 2015, 10:50 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

review comments addressed.


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-25 Thread Jojy Varghese

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

(Updated Sept. 25, 2015, 6:33 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

rebased with master


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-22 Thread Jojy Varghese

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

(Updated Sept. 22, 2015, 11:37 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

addressed review comments.


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
1b0c30450a07d01d1a38d460bc7d921c80be7e3b 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38443]

All tests passed.

- Mesos ReviewBot


On Sept. 17, 2015, 5:06 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 17, 2015, 5:06 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Jojy Varghese

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

(Updated Sept. 17, 2015, 5:06 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Added guard for array sizes.


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
1b0c30450a07d01d1a38d460bc7d921c80be7e3b 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Jojy Varghese


> On Sept. 17, 2015, 2:54 a.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/docker/registry_client.cpp, line 499
> > 
> >
> > If history array contains non JSON Object from the response I think it 
> > will just crash the slave.
> > 
> > I realize in other places too, this becomes a bit tricky when docker 
> > registry changes their impl, or when user is targeting some other version 
> > of the registry I tihnk we shouldn't crash the whole slave.

if you mean the array sizes differences between history and fslayers then i 
have fixed the patch for that.


- Jojy


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


On Sept. 17, 2015, 5:06 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 17, 2015, 5:06 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Jojy Varghese

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

(Updated Sept. 18, 2015, 1:17 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Added guard for as. Also moved arguments around.


Repository: mesos


Description
---

Added layerid information to ManifestResponse


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
1b0c30450a07d01d1a38d460bc7d921c80be7e3b 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-17 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38443]

All tests passed.

- Mesos ReviewBot


On Sept. 18, 2015, 1:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 18, 2015, 1:17 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



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 (line 499)


If history array contains non JSON Object from the response I think it will 
just crash the slave.

I realize in other places too, this becomes a bit tricky when docker 
registry changes their impl, or when user is targeting some other version of 
the registry I tihnk we shouldn't crash the whole slave.


- Timothy Chen


On Sept. 17, 2015, 12:27 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38443/
> ---
> 
> (Updated Sept. 17, 2015, 12:27 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added layerid information to ManifestResponse
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 1b0c30450a07d01d1a38d460bc7d921c80be7e3b 
> 
> Diff: https://reviews.apache.org/r/38443/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>