Skip to content
This repository has been archived by the owner on Nov 7, 2024. It is now read-only.

Look at migrating docker plugin to use Docker SDK for Python #87

Closed
cjh1 opened this issue Jan 13, 2017 · 13 comments
Closed

Look at migrating docker plugin to use Docker SDK for Python #87

cjh1 opened this issue Jan 13, 2017 · 13 comments

Comments

@cjh1
Copy link
Contributor

cjh1 commented Jan 13, 2017

Move away from popen to https://github.com/docker/docker-py

@zachmullen
Copy link
Member

👍

Looks like this communicates directly with the service over HTTP rather than using a subprocess, which is cool. We'd have to investigate how well its API supports streaming IO.

@cjh1
Copy link
Contributor Author

cjh1 commented Jan 13, 2017

Yes, talking directly to the service seems like a nice step up from using subprocess. When you say "streaming IO" are you taking about the support we currently have for pipes?

@zachmullen
Copy link
Member

Yeah, the ability to do non-blocking reads from pipes (a la select).

@kotfic
Copy link
Contributor

kotfic commented Jan 16, 2017

was just poking around a little dockerpty may be relevant.

@cjh1
Copy link
Contributor Author

cjh1 commented Jan 19, 2017

Recording quote from @zachmullen "we should see about using more modern tooling for docker garbage collection in lieu of the current docker-gc script, which is a third party tool that was the industry standard for a while." Was thinking the same ...

@cjh1
Copy link
Contributor Author

cjh1 commented Jan 19, 2017

Some possible alternative to docker-gc script:

https://github.com/Yelp/docker-custodian (uses docker-py )

@manthey
Copy link
Member

manthey commented Jan 20, 2017

Docker-py has just recently release a version 2.0. Prior to version 2.0, it is docker-py on pip, now it is is just docker. There are substantial breaking changes between the two. We use version 1.9 in the slicer_cli_web and HistomicsTK projects, so if we use version 2, we may want to update those as well.

It almost seems like less work to use the command line interface than docker-py, especially for volumes. Version 2 is a little cleaner than version 1.9. The transition wouldn't be hard.

And, our GC task should run out-of-band; it seems wasteful to delay sending job results until the GC has run, especially when that GC can take a long time.

@cjh1
Copy link
Contributor Author

cjh1 commented Jan 20, 2017

Docker-py has just recently release a version 2.0. Prior to version 2.0, it is docker-py on pip, now it is is just docker. There are substantial breaking changes between the two. We use version 1.9 in the slicer_cli_web and HistomicsTK projects, so if we use version 2, we may want to update those as well.

It almost seems like less work to use the command line interface than docker-py, especially for volumes. Version 2 is a little cleaner than version 1.9. The transition wouldn't be hard.

@manthey Are you saying that the move docker-py isn't worth it despite being able to talk directly to the docker daemon rather than through a sub process?

And, our GC task should run out-of-band; it seems wasteful to delay sending job results until the GC has run, especially when that GC can take a long time.

I agree a celery periodic task could be used for this

@manthey
Copy link
Member

manthey commented Jan 20, 2017

@cjh1 I don't think switching to docker-py will reduce code complexity. Version 2 won't increase it much, but version 1.x will reduce readability. If it gains us robustness or maintainability, then switch can still be worth while.

@cjh1
Copy link
Contributor Author

cjh1 commented Jan 20, 2017

It would reduce the number of processes used to run a docker task by one, which might help with scalability.

@zachmullen
Copy link
Member

As part of this, we should try to mitigate the busy loop behavior while we wait on the docker container to finish.

@jcfr
Copy link
Contributor

jcfr commented Jan 23, 2017 via email

@cjh1
Copy link
Contributor Author

cjh1 commented May 12, 2017

Fixed by #96

@cjh1 cjh1 closed this as completed May 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants