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.

Sign up for GitHub

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

Jump to bottom

[INTEL MKL] Fix proxy info during docker builds #29560

Merged
merged 6 commits into from Jun 19, 2019
Merged

[INTEL MKL] Fix proxy info during docker builds #29560

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
239d559
Fix proxy info during docker builds
ashahba Jun 8, 2019
3e931cc
Merge branch 'master' of github.com:ashahba/tensorflow into ashahba/p…
ashahba Jun 13, 2019
f8940f0
Add comments for proxy info per code review
ashahba Jun 13, 2019
c2c129b
Merge branch 'master' of github.com:ashahba/tensorflow into ashahba/p…
ashahba Jun 14, 2019
7b61e31
Added comment for uppercase proxies per code review
ashahba Jun 14, 2019
798109d
Merge branch 'master' of github.com:ashahba/tensorflow into ashahba/p…
ashahba Jun 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions tensorflow/tools/ci_build/linux/mkl/build-dev-container.sh
View file
Open in desktop
Expand Up @@ -80,14 +80,18 @@ function build_container()
shift
TF_DOCKER_BUILD_ARGS=("${@}")

# Add the proxy info build args
# Add the proxy info build args. This will be later on passed to docker as
# --build-arg so that users behind corporate proxy can build the images
TF_DOCKER_BUILD_ARGS+=("--build-arg http_proxy=${http_proxy}")
TF_DOCKER_BUILD_ARGS+=("--build-arg https_proxy=${https_proxy}")
TF_DOCKER_BUILD_ARGS+=("--build-arg socks_proxy=${socks_proxy}")
TF_DOCKER_BUILD_ARGS+=("--build-arg no_proxy=${no_proxy}")
TF_DOCKER_BUILD_ARGS+=("--build-arg HTTP_PROXY=${http_proxy}")
TF_DOCKER_BUILD_ARGS+=("--build-arg SOCKS_PROXY=${socks_proxy}")
TF_DOCKER_BUILD_ARGS+=("--build-arg NO_PROXY=${no_proxy}")
# In general having uppercase proxies is a good idea because different
# applications running inside Docker may only honor uppercase proxies
TF_DOCKER_BUILD_ARGS+=("--build-arg HTTP_PROXY=${HTTP_PROXY}")
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment explaining that we need the uppercase versions because the lowercase ones don't cover all the case (or something).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @penpornk to best of my knowledge I added a comment to describe why we need both set of proxies 🙂

Copy link
Member

Choose a reason for hiding this comment

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

Thank you very much, @ashahba! :)

TF_DOCKER_BUILD_ARGS+=("--build-arg HTTPS_PROXY=${HTTPS_PROXY}")
TF_DOCKER_BUILD_ARGS+=("--build-arg SOCKS_PROXY=${SOCKS_PROXY}")
TF_DOCKER_BUILD_ARGS+=("--build-arg NO_PROXY=${NO_PROXY}")

#Add --config=v2 build arg for TF v2
if [[ ${BUILD_TF_V2_CONTAINERS} == "no" ]]; then
Expand Down