Skip to content

fix: flip master to script mode #222

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

Closed
wants to merge 1 commit into from

Conversation

icywang86rui
Copy link
Contributor

Issue #, if available:

Description of changes:
This change copies the tip of script-mode branch.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@icywang86rui icywang86rui requested a review from mvsusp July 8, 2019 21:37

#. `Getting Started <#getting-started>`__
#. `Building your Image <#building-your-image>`__
#. `Running the tests <#running-the-tests>`__
Copy link
Contributor

Choose a reason for hiding this comment

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

why not let it be automatically generated anymore?

@@ -65,10 +66,10 @@ The "base" Dockerfile encompass the installation of the framework and all of the
needed. It is needed before building image for TensorFlow 1.8.0 and before.
Building a base image is not required for images for TensorFlow 1.9.0 and onwards.

Tagging scheme is based on <tensorflow_version>-<processor>-<python_version>. (e.g. 1.4
Tagging scheme is based on <tensorflow_version>-<processor>-<python_version>. (e.g. 1.4
.1-cpu-py2)
Copy link
Contributor

Choose a reason for hiding this comment

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

that's a weird line break haha

@@ -272,10 +237,10 @@ Functional Tests
Functional tests require your Docker image to be within an `Amazon ECR repository <https://docs
.aws.amazon.com/AmazonECS/latest/developerguide/ECS_Console_Repositories.html>`__.

The `docker-base-name` is your `ECR repository namespace <https://docs.aws.amazon
The Docker-base-name is your `ECR repository namespace <https://docs.aws.amazon
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the dashes from "Docker base name"

.com/AmazonECR/latest/userguide/Repositories.html>`__.

The `instance-type` is your specified `Amazon SageMaker Instance Type
The instance-type is your specified `Amazon SageMaker Instance Type
Copy link
Contributor

Choose a reason for hiding this comment

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

s/instance-type/instance type

Copy link
Contributor

@laurenyu laurenyu left a comment

Choose a reason for hiding this comment

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

also - will the documentation be updated in a future PR that explains what's going on here? (e.g. what if someone is looking for TF 1.6?)

@@ -1,19 +1,38 @@
import os
# Copyright 2017-2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

2019

@laurenyu
Copy link
Contributor

closing in favor of #234

@laurenyu laurenyu closed this Oct 15, 2019
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

Successfully merging this pull request may close these issues.

2 participants