aws / sagemaker-tensorflow-serving-container

A TensorFlow Serving solution for use in SageMaker. This repo is now deprecated.
Apache License 2.0
172 stars 101 forks source link

Request: deploy base images to docker hub #8

Open tedivm opened 5 years ago

tedivm commented 5 years ago

It would be great if I could just extend the existing containers instead of having to clone this repo and essentially fork the project as the basis for my own. Building and pushing the containers to dockerhub would allow people to extend those containers pretty easily.

mvsusp commented 5 years ago

Hi @tedivm,

Your suggestion is great, we actually have similar plans in our roadmap. I will keep this ticket open as a feature request.

Thanks for using SageMaker.

jesterhazy commented 5 years ago

@tedivm in the meantime there is a way to accomplish this even though our images are in ECR.

  1. create a docker file that extends one of our images:
FROM 520713654638.dkr.ecr.us-west-2.amazonaws.com/sagemaker-tensorflow-serving:1.12-cpu

... other dockerfile commands ...
  1. login to our ECR registry

$(aws ecr get-login --no-include-email --registry-id 520713654638)

  1. build your docker image

docker build ....

Docker will automatically pull our image from ECR, just like it would from dockerhub. You just need to log in first.

You can also login and the pull our image first. The the image will be cached locally, after which you won't need to do the ecr login before building your derived image.

$(aws ecr get-login --no-include-email --registry-id 520713654638)
docker pull 520713654638.dkr.ecr.us-west-2.amazonaws.com/sagemaker-tensorflow-serving:1.12-cpu

We tag our images with \<TFS version>-\<processor>. As of now the valid tags are:

tedivm commented 5 years ago

Since this wasn't too complicated I went ahead and set my fork to push to docker hub. I've made a pull request here in case you want it as well- I'm not sure how you feel about integrating with things like CircleCi, but you can at least use my bash script if you don't want the CircleCI config as well.

nadiaya commented 5 years ago

Thank you for your interest and the contribution. Since SageMaker is not integrated with docker hub we will update the issue as soon as we figure out what would be the right way to incorporate your contribution into this project.

joshburt commented 4 years ago

Looks like I'm a bit late the the party but I thought I'd chime in as well.

The work-around mentioned here is sufficient for local development and integration tests. Once you have the image cached locally it can be started up using a similar script to https://github.com/aws/sagemaker-tensorflow-serving-container/blob/master/scripts/start.sh

Thanks for the work-around and test coverage code for review. It was very helpful!