You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Isabel Jimenez <co...@isabeljimenez.com> on 2015/07/02 10:58:50 UTC
Review Request 36128: Change docker version parsing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36128/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Timothy Chen.
Bugs: MESOS-2986
https://issues.apache.org/jira/browse/MESOS-2986
Repository: mesos-incubating
Description
-------
We currently use `docker version` to get Docker version, in Docker master branch and soon in Docker 1.8 [1] the output for this command changes. The solution for now will be to use the unchanged `docker --version` output, in the long term we should consider stop using the cli and use the API instead.
[1] https://github.com/docker/docker/pull/14047
Diffs
-----
src/docker/docker.cpp cbcb16e
Diff: https://reviews.apache.org/r/36128/diff/
Testing
-------
make check
Thanks,
Isabel Jimenez
Re: Review Request 36128: Change docker version parsing
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36128/#review90257
-----------------------------------------------------------
Ship it!
Ship It!
- Timothy Chen
On July 2, 2015, 8:58 a.m., Isabel Jimenez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36128/
> -----------------------------------------------------------
>
> (Updated July 2, 2015, 8:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Timothy Chen.
>
>
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> We currently use `docker version` to get Docker version, in Docker master branch and soon in Docker 1.8 [1] the output for this command changes. The solution for now will be to use the unchanged `docker --version` output, in the long term we should consider stop using the cli and use the API instead.
>
> [1] https://github.com/docker/docker/pull/14047
>
>
> Diffs
> -----
>
> src/docker/docker.cpp cbcb16e
>
> Diff: https://reviews.apache.org/r/36128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Isabel Jimenez
>
>
Re: Review Request 36128: Change docker version parsing
Posted by Isabel Jimenez <co...@isabeljimenez.com>.
> On July 2, 2015, 5:47 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 189
> > <https://reviews.apache.org/r/36128/diff/1/?file=998179#file998179line189>
> >
> > It's a bit harder to understand how this works.
> >
> > Looks like the new output is:
> > Client:
> > Version: 1.8.0-dev
> > ........
> >
> > I don't really see any commas, and don't know how is the first line skipped.
> >
> > But I think most importantly we need to support both < 1.8 and >= 1.8 and do different parsing since not all users will be upgrading.
> >
> > So I think if there is a way to detect this or have one way to parse both that will good.
> >
> > Either way we should put in the comments what does the output look like.
I chose to use `docker --version` instead of `docker version` specifically for that reason, the output has not changed and isn't going to. This means that by getting the version with this command we will support both < 1.8 and >= 1.8
- Isabel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36128/#review90254
-----------------------------------------------------------
On July 2, 2015, 8:58 a.m., Isabel Jimenez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36128/
> -----------------------------------------------------------
>
> (Updated July 2, 2015, 8:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Timothy Chen.
>
>
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> We currently use `docker version` to get Docker version, in Docker master branch and soon in Docker 1.8 [1] the output for this command changes. The solution for now will be to use the unchanged `docker --version` output, in the long term we should consider stop using the cli and use the API instead.
>
> [1] https://github.com/docker/docker/pull/14047
>
>
> Diffs
> -----
>
> src/docker/docker.cpp cbcb16e
>
> Diff: https://reviews.apache.org/r/36128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Isabel Jimenez
>
>
Re: Review Request 36128: Change docker version parsing
Posted by Timothy Chen <tn...@apache.org>.
> On July 2, 2015, 5:47 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 189
> > <https://reviews.apache.org/r/36128/diff/1/?file=998179#file998179line189>
> >
> > It's a bit harder to understand how this works.
> >
> > Looks like the new output is:
> > Client:
> > Version: 1.8.0-dev
> > ........
> >
> > I don't really see any commas, and don't know how is the first line skipped.
> >
> > But I think most importantly we need to support both < 1.8 and >= 1.8 and do different parsing since not all users will be upgrading.
> >
> > So I think if there is a way to detect this or have one way to parse both that will good.
> >
> > Either way we should put in the comments what does the output look like.
>
> Isabel Jimenez wrote:
> I chose to use `docker --version` instead of `docker version` specifically for that reason, the output has not changed and isn't going to. This means that by getting the version with this command we will support both < 1.8 and >= 1.8
Got it thanks.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36128/#review90254
-----------------------------------------------------------
On July 2, 2015, 8:58 a.m., Isabel Jimenez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36128/
> -----------------------------------------------------------
>
> (Updated July 2, 2015, 8:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Timothy Chen.
>
>
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> We currently use `docker version` to get Docker version, in Docker master branch and soon in Docker 1.8 [1] the output for this command changes. The solution for now will be to use the unchanged `docker --version` output, in the long term we should consider stop using the cli and use the API instead.
>
> [1] https://github.com/docker/docker/pull/14047
>
>
> Diffs
> -----
>
> src/docker/docker.cpp cbcb16e
>
> Diff: https://reviews.apache.org/r/36128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Isabel Jimenez
>
>
Re: Review Request 36128: Change docker version parsing
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36128/#review90254
-----------------------------------------------------------
src/docker/docker.cpp (line 189)
<https://reviews.apache.org/r/36128/#comment143266>
It's a bit harder to understand how this works.
Looks like the new output is:
Client:
Version: 1.8.0-dev
........
I don't really see any commas, and don't know how is the first line skipped.
But I think most importantly we need to support both < 1.8 and >= 1.8 and do different parsing since not all users will be upgrading.
So I think if there is a way to detect this or have one way to parse both that will good.
Either way we should put in the comments what does the output look like.
- Timothy Chen
On July 2, 2015, 8:58 a.m., Isabel Jimenez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36128/
> -----------------------------------------------------------
>
> (Updated July 2, 2015, 8:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Timothy Chen.
>
>
> Bugs: MESOS-2986
> https://issues.apache.org/jira/browse/MESOS-2986
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> We currently use `docker version` to get Docker version, in Docker master branch and soon in Docker 1.8 [1] the output for this command changes. The solution for now will be to use the unchanged `docker --version` output, in the long term we should consider stop using the cli and use the API instead.
>
> [1] https://github.com/docker/docker/pull/14047
>
>
> Diffs
> -----
>
> src/docker/docker.cpp cbcb16e
>
> Diff: https://reviews.apache.org/r/36128/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Isabel Jimenez
>
>