Make Docker::Container.new public or make possible to create from conatiner id about docker-api HOT 13 CLOSED

@nahiluhmot can you weigh in here?

from docker-api.

Docker::Image.new is intended to be private, but regardless I'm not sure what the use case would be for creating an instance directly via an id or name. You could try Docker::Container.all.select { |container| container.id == YOUR_ID }.first. The purpose of keeping the .new methods private is to ensure that the instance of Docker::Image or Docker::Container actually corresponds to an existing image or container, respectively.

from docker-api.

My use case is that i store the container id to know which container is the current instance of an "app" so that i later on can replace it when deploying a new version. Dokku does the same thing. By being able to create an Docker::Container instance directly from an id (or name) i don't have to fetch all containers to find it.

But note that making new private in the client API does not protect against a container or image disappearing from the docker server while a docker-api instance is alive. You still need to take care of not found exceptions etc if you want to be careful.

If you think this is a bad idea i could continue to fetch all containers, not a big problem really... or i could use .send(:new, ...) 😄

from docker-api.

@nahiluhmot maybe a convenience Container.for_id or for_name method which returned nil if not found would be a good step here?

from docker-api.

@bfulton That would work. Currently i do a ugly .send(:new, ...) and then directly do .json and rescue Excon::Errors::NotFound. Would be nice if that is done in some helper method.

from docker-api.

I see, I agree with @bfulton's response, that would keep some of the assumptions made in the gem. Of course, a parallel method would have to be implemented in Docker::Image for consistency.

from docker-api.

And make Docker::Image.new private?

The for_id and for_name could use the private new right? and then do something that check if it really exist. Fetching all containers each time seems quite wasteful and not what i would except as a API user.

What assumptions does the gem make btw?

from docker-api.

Combine this with #62? for_name would kind of depend on creating named container if we don't write the test to do it externally somehow.

from docker-api.

@wader I'm not sure there is any other way to query the server to find an image other than to just get all. Docker truncates a lot of data when responding to other method calls and quite often just returns the image Id.

Since containers are returned with some meta data (like names), .all should need to first add that data to the Container object (similar to what Image does with .all). As soon as we can get the Names from the Containers then we can adequately test this and as a side effect create .for_name.

from docker-api.

@tlunter docker API 0.7.0 and higher supports referencing a container by name. Yeah i noticed the id truncation some days ago, thought it was docker-api first but then saw how docker replied to some requests.

You want for_id and for_name to automatically fetch information when created and fail if container do not exist? currently when i use the .send(:new, ..) hack i take care of the not found later on when i start to send requests to the server. I need to take care of that case anyway as the container could be removed from the server at any time.

#62 have a test case now btw.

from docker-api.

Docker-api generally handles failures from Docker by just returning certain Docker::Error exceptions. The only problem with dealing with possible errors later on is that Docker doesn't have the easiest errors to react to.

I think calling for_id and for_name should do something similar to what @nahiluhmot had recommended with a Docker::Container.all.select { |c| c.x == x }.first. I think this is the best way since this seems to be the only way to get data from Docker about a container.

from docker-api.

@wader Getting containers by name is fixed by #65. Do you still need images?

from docker-api.

@tlunter Docker::Image.new is public atm so it is kind of already possible. But maybe a get that fetches information about the image (and fails if not found) would be nice.

from docker-api.