Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker 0.8 compatibility #159

Closed
mpetazzoni opened this issue Feb 5, 2014 · 17 comments
Closed

Docker 0.8 compatibility #159

mpetazzoni opened this issue Feb 5, 2014 · 17 comments
Milestone

Comments

@mpetazzoni
Copy link
Contributor

Placeholder issue for investigating, testing and validating docker-py with Docker 0.8 as well as making the necessary changes where required to make it all work.

@ghost ghost assigned mpetazzoni Feb 5, 2014
@mpetazzoni
Copy link
Contributor Author

From @jakedt:

It looks like it's only broken if you tell the client to user API revision
1.9, which switches to the json block generating StreamFormatter in the
daemon. It doesn't have newlines between the json chunks, and i couldn't
get requests to dump chunks with iter_chunks either.

@ibuildthecloud
Copy link
Contributor

Docker 0.8 has seemingly broken the <=1.6 version of the images API:

curl http://localhost:8085/v1.8/images/json
[
  {
    "Created":1391493237,
    "Id":"94041e8be8ec54d6557bf88e02ce00cd1204f401d955fd2bb2840d2d3c1ee027",
    "ParentId":"7527bf080f7fa1f90fd477bc4dab20d16d4b8a2bc6e3f5d27aca69dba4487f35",
    "RepoTags":["ibuildthecloud/dstack:latest"],
    "Size":0,
    "VirtualSize":543223874
  }
]
curl http://localhost:8085/v1.6/images/json
[
  {
    "Created":1391493237,
    "ID":"",
    "Repository":"ibuildthecloud/dstack",
    "Size":0,
    "Tag":"latest",
    "VirtualSize":543223874
  }
]

@ibuildthecloud
Copy link
Contributor

May not be obvious, its broken because 'Id' is "ID" I found this is a simple bug in docker v0.8 and I'll submit a PR there shortly

@jakedt
Copy link

jakedt commented Feb 5, 2014

The Id is ID and it's also missing. Additionally, the format for Repository is different and is now a list in RepoTags.

@mpetazzoni
Copy link
Contributor Author

Ah yes sorry overlooked that. I actually stumbled upon that problem in Maestro recently. I yields a slightly bigger question though for docker-py as to how we want to handle multiple versions of the Docker remote API in our code. It will quickly become very messy. Is this something that could be solved by inheritance, having different implementations for the various versions of the API, overloading on top of each other?

@shin-
Copy link
Contributor

shin- commented Feb 6, 2014

Just merged #161 (thanks @mpetazzoni ! :))

Is this something that could be solved by inheritance, having different implementations for the various versions of the API, overloading on top of each other?

We could look into something like that, or maybe just decide that the next version of docker-py will only work with docker 0.8.0+. See discussion in #76 for more on backwards compatibility.

Also, once we've made sure that docker-py works with docker 0.8.0, we'll release a new version.

@mpetazzoni
Copy link
Contributor Author

Thanks. I agree. We should definitely release a 0.8.0-compatible version soon (see the 0.2.4 milestone I created recently). I also think we should freeze docker-py versions that match Docker versions, hopefully as soon as possible after each Docker release. What do you think?

@shin-
Copy link
Contributor

shin- commented Feb 18, 2014

Is there a reason we're holding off on making 1.9 the default API version? I just updated the integration tests and they pass fine with both 1.8 and 1.9 (but they're admittedly not 100% coverage). What else do we need to make this release happen? I'll start working on the ChangeLog and README updates.

cc @mpetazzoni

@mpetazzoni
Copy link
Contributor Author

We might need to update the build() method, since the endpoint changed quite a bit in recent Docker versions.

@shin-
Copy link
Contributor

shin- commented Feb 19, 2014

build works. It's not returning the produced image ID like it used to with more recent versions of the API, but that's something we can restore if/when we start properly parsing JSON streams. If that's the only thing, I think we can release 0.3.0 now.

@mpetazzoni
Copy link
Contributor Author

I think we should fix the JSON stream parsing first. I'll take a crack at it today. If I don't get it working, we can release 0.3.0 tomorrow. Sounds good?

@shin-
Copy link
Contributor

shin- commented Feb 19, 2014

Sure, sounds good to me.

@mpetazzoni
Copy link
Contributor Author

See #167 . There isn't much we can do to parse this correctly as is without being stupidly O(n^2) because the JSON objects are not delimited.

@jakedt
Copy link

jakedt commented Feb 19, 2014

I don't think it has to be O(n^2), but it would require a custom json parser. There's no reason a properly coded parser couldn't take in chunks of text while tracking how many "closing brackets" it's waiting for, and emitting an object when it receives a complete properly formed json object, while preserving the "remainder" in the buffer for the next object.

More concerning to me is that iter_chunks wasn't working regardless of how small I made the chunk size. Does this mean that the daemon is delivering all results as a single chunk?

@mpetazzoni
Copy link
Contributor Author

@jakedt Not sure why we never got it to work before, but with my changes in #167 I have it working on API version 1.9 without issues. We were just not using the right stream helper. Feel free to give https://github.com/mpetazzoni/docker-py/tree/build-output a try and let me know.

@shin-
Copy link
Contributor

shin- commented Feb 24, 2014

tagged 0.3.0, and released on pypi as of now. I'm closing this issue, if there's anything that still needs to be discussed feel free to reopen it though.

@shin- shin- closed this as completed Feb 24, 2014
@mpetazzoni
Copy link
Contributor Author

Yay!

@mpetazzoni mpetazzoni removed their assignment Jul 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants