Skip to content

Conversation

zhouzhuojie
Copy link
Contributor

@zhouzhuojie zhouzhuojie commented Jul 23, 2021

Stack from ghstack:

This PR adds a squid proxy that's deployed dedicated for PyTorch CI. Initially we only roll out to GHA, and if things are ok we will extend this to circleci tests if necessary.

http_proxy and https_proxy are compatible with the following http clients:

  • curl
  • wget
  • python

Existing cache policy:

refresh_pattern -i .(7z|deb|rpm|exe|zip|tar|tgz|gz|ram|rar|bin|tiff|bz2|run|csv|sh)$ 1440 80% 2880

It uses the standard squid refresh_pattern for cache requests. In our setup, we tried
to cache at least (1440 minutes - 1 day) and at max (2880 minutes - 2 days), with
last-modified factor 80% (squid doc). Please refer to pytorch/test-infra for details.

Right now, it only applies to the build and test step, to limit the scope and make sure build and test are more reliable with egress cache.

Differential Revision: D29892919

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 23, 2021

🔗 Helpful links

💊 CI failures summary and remediations

As of commit fbe2640 (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.

Click here to manually regenerate this comment.

@malfet
Copy link
Contributor

malfet commented Jul 23, 2021

Can you please add a bit more details to PR description? I.e. why both http_proxy and HTTP_PROXY is needed. Same about use_proxy=on

Copy link
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

I think you are missing -e option in bazel workflow template



This PR adds a squid proxy that's deployed dedicated for PyTorch CI. Initially we only roll out to GHA, and if things are ok we will extend this to circleci tests if necessary.

`http_proxy` and `https_proxy` are compatible with the following http clients:

- curl
- wget
- python



[ghstack-poisoned]
zhouzhuojie added a commit that referenced this pull request Jul 23, 2021
ghstack-source-id: d425154
Pull Request resolved: #62103


This PR adds a squid proxy that's deployed dedicated for PyTorch CI. Initially we only roll out to GHA, and if things are ok we will extend this to circleci tests if necessary.

`http_proxy` and `https_proxy` are compatible with the following http clients:

- curl
- wget
- python


Existing cache policy:

```
refresh_pattern -i .(7z|deb|rpm|exe|zip|tar|tgz|gz|ram|rar|bin|tiff|bz2|run|csv|sh)$ 1440 80% 2880
```

It uses the standard squid refresh_pattern for cache requests. In our setup, we tried
to cache at least (1440 minutes - 1 day) and at max (2880 minutes - 2 days), with
last-modified factor 80% ([squid doc](http://www.squid-cache.org/Doc/config/refresh_pattern/)). Please refer to [pytorch/test-infra](https://github.com/pytorch/test-infra/tree/master/aws/websites/squid-proxy) for details.

Right now, it only applies to the `build` and `test` step, to limit the scope and make sure build and test are more reliable with egress cache.

[ghstack-poisoned]
zhouzhuojie added a commit that referenced this pull request Jul 23, 2021
ghstack-source-id: a75edaf
Pull Request resolved: #62103
@zhouzhuojie
Copy link
Contributor Author

@zhouzhuojie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Member

@seemethere seemethere left a comment

Choose a reason for hiding this comment

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

LGTM

Note entirely sure if case matters for these calls but the Docker documentation does suggest using all uppercase for the env variable names:

Screen Shot 2021-07-26 at 11 35 10 AM

From https://docs.docker.com/network/proxy/

Copy link
Contributor

@driazati driazati left a comment

Choose a reason for hiding this comment

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

lgtm too, just curious if squid proxy is down / unreachable for some reason all network accesses will timeout / fail right?

@zhouzhuojie
Copy link
Contributor Author

lgtm too, just curious if squid proxy is down / unreachable for some reason all network accesses will timeout / fail right?

Yes, the proxy is in the critical path, and we will beef up the monitoring of it

@zhouzhuojie
Copy link
Contributor Author

LGTM

Note entirely sure if case matters for these calls but the Docker documentation does suggest using all uppercase for the env variable names:

Screen Shot 2021-07-26 at 11 35 10 AM

From https://docs.docker.com/network/proxy/

The common http clients usually respect both format, and lowercase is kind of the default. Found a good summary on this topic

https://about.gitlab.com/blog/2021/01/27/we-need-to-talk-no-proxy/

image

@facebook-github-bot
Copy link
Contributor

@zhouzhuojie merged this pull request in e63160d.

@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by f16102f.

@facebook-github-bot facebook-github-bot deleted the gh/zhouzhuojie/5/head branch July 30, 2021 14:27
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.

6 participants