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

Fix pulling with container create api #2636

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BeardedDonut
Copy link

Fix the pulling issue stated in #2635. Basically, similar to docker create it should pull the image if it does not exist locally.

Fix the pulling issue stated in docker#2635. Basically, similar to `docker
create` it should pull the image if it does not exist locally.

Signed-off-by: Navid Ali Pour <navid9675@gmail.com>
@dan-hipschman
Copy link

I would like to see the Python SDK and Docker CLI behavior in sync, so support this PR, but should point out that the current difference is documented:

def create(self, image, command=None, **kwargs):
"""
Create a container without starting it. Similar to ``docker create``.
Takes the same arguments as :py:meth:`run`, except for ``stdout``,
``stderr``, and ``remove``.
Returns:
A :py:class:`Container` object.
Raises:
:py:class:`docker.errors.ImageNotFound`
If the specified image does not exist.

This PR should at least update the documentation and ContainerCollection.run (which can call create twice). Then the maintainers can decide whether they want to change the documented behavior.

@BeardedDonut
Copy link
Author

@dan-hipschman-od Thanks for your comment, I just didn't get this part would you please clarify a bit.

This PR should at least update the documentation and ContainerCollection.run (which can call create twice). Then the maintainers can decide whether they want to change the documented behavior.

Thanks.

@dan-hipschman
Copy link

Yea, I was just saying two things:

  • Could you update the documentation of ContainerCollection.create so it says it will try to pull the image if it doesn't exist locally?
  • And ContainerCollection.run currently calls create and assumes it will not pull the image, so run itself catches ImageNotFound and tries to pull the image (see below). Since you're changing create to pull the image, then run can be simplified:
    try:
    container = self.create(image=image, command=command,
    detach=detach, **kwargs)
    except ImageNotFound:
    self.client.images.pull(image, platform=platform)
    container = self.create(image=image, command=command,
    detach=detach, **kwargs)

Copy link
Member

@milas milas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

I'm going to echo the great feedback already provided by @dan-hipschman-od:

Yea, I was just saying two things:

  • Could you update the documentation of ContainerCollection.create so it says it will try to pull the image if it doesn't exist locally?
  • And ContainerCollection.run currently calls create and assumes it will not pull the image, so run itself catches ImageNotFound and tries to pull the image (see below). Since you're changing create to pull the image, then run can be simplified:
    try:
    container = self.create(image=image, command=command,
    detach=detach, **kwargs)
    except ImageNotFound:
    self.client.images.pull(image, platform=platform)
    container = self.create(image=image, command=command,
    detach=detach, **kwargs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants