Fix windows ci squid env #62353
Fix windows ci squid env #62353
Conversation
zhouzhuojie
requested review from
driazati,
janeyx99 and
seemethere
as code owners
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 1de23a2 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
|
@zhouzhuojie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| @@ -37,12 +40,12 @@ env: | |||
| VS_VERSION: "16.8.6" | |||
| VC_YEAR: "2019" | |||
| ALPINE_IMAGE: "308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine" | |||
| no_proxy: !{{ squid_no_proxy }} | |||
There was a problem hiding this comment.
Any reason we don't set http_proxy at the workflow level too?
There was a problem hiding this comment.
I'm trying to simulate what linux CI is doing is to limit the proxy to only build and test jobs, linux CI runs those in docker, windows CI runs in plain steps, thus the differences.
| @@ -60,6 +60,7 @@ name: Bazel Linux CI (!{{ build_environment }}) | |||
| -e CUSTOM_TEST_ARTIFACT_BUILD_DIR \ | |||
| -e SKIP_SCCACHE_INITIALIZATION=1 \ | |||
| -e TORCH_CUDA_ARCH_LIST \ | |||
| -e http_proxy="!{{squid_proxy}}" -e https_proxy="!{{squid_proxy}}" -e no_proxy="!{{squid_no_proxy}}" \ | |||
There was a problem hiding this comment.
These (squid_proxy, etc.) look like they're not defined for this workflow? Since they're duplicated across templates anyways it makes sense to pull them out into generate_ci_workflows.py so they're more consolidated
There was a problem hiding this comment.
this bazel workflow is an extension of linux workflow, but with combined build + test jobs into one, so many environment setup are defined in linux jinja2.
zhouzhuojie
force-pushed
the
fix-windows-ci-squid-env
branch
2 times, most recently
from
794583c to
8a606e5
Compare
|
@zhouzhuojie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
zhouzhuojie
force-pushed
the
fix-windows-ci-squid-env
branch
from
8a606e5 to
1de23a2
Compare
| @@ -1,4 +1,8 @@ | |||
| {%- set exclude_test = exclude_test|default(false) -%} | |||
There was a problem hiding this comment.
exclude_test is already set as False by default, so no need to set again here
|
@zhouzhuojie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@zhouzhuojie merged this pull request in 7157ad4. |
This is a re-land of #62244, noticeable changes are