Skip to content

Fix infinite loop in _validate_not_a_forked_repo() #62072

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

Conversation

malfet
Copy link
Contributor

@malfet malfet commented Jul 23, 2021

Increase page_idx in the loop rather than outside of it
Break from the loop when receive empty response as it means there are no more items to fetch via pagination request

Also, add options to use provided github token (via GITHUB_TOKEN environment variable)

Fixes failure with "Rate Limit Exceeded" when doing something like torch.hub.list("pytorch/test-infra:dsf")

Fixes #61755

malfet added 2 commits July 22, 2021 18:25
Increase `page_idx` in the loop rather than outside of it
Break from the loop when receive empty response as it means there are no
more items to fetch via pagination request

Fixes failure with "Rate Limit Exceeded" when doing something like `torch.hub.list("pytorch/test-infra:dsf")`
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 23, 2021

🔗 Helpful links

💊 CI failures summary and remediations

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@malfet malfet requested review from mthrok and anjali411 July 23, 2021 02:01
Copy link
Contributor

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

LGTM

if not response:
break
for br in response:
if br['name'] == branch or br['commit']['sha'].startswith(branch):
Copy link
Contributor

@mthrok mthrok Jul 23, 2021

Choose a reason for hiding this comment

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

Note: Wonder if we should check the existence of the key. It's very unlikely to happen but we could give better error message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point let me address that

@malfet malfet force-pushed the malfet/fix-loop-in-_validate_not_a_forked_repo branch from acdfcfc to 981a022 Compare July 23, 2021 23:43
@malfet
Copy link
Contributor Author

malfet commented Jul 23, 2021

Moved skipped_validation to #62139

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@malfet merged this pull request in b6d10a3.

facebook-github-bot pushed a commit that referenced this pull request Jul 27, 2021
Summary:
Split from #62072

Pull Request resolved: #62139

Reviewed By: mthrok

Differential Revision: D29891497

Pulled By: malfet

fbshipit-source-id: 5c0baf53a2acf8f95062bd001457e1f936011529
@gchanan gchanan added this to the 1.9.1 milestone Aug 30, 2021
@malfet malfet deleted the malfet/fix-loop-in-_validate_not_a_forked_repo branch September 8, 2021 01:54
malfet added a commit to pytorch/builder that referenced this pull request Sep 8, 2021
Example:
```
 ./github_analyze.py --missing-in-branch --branch release/1.9 --milestone-id 23
URL;Title;Status
https://github.com/pytorch/pytorch/pull/64036;[torch/elastic] Pretty print the failure message captured by @record;closed
pytorch/pytorch#63953 torch.distributed.run OMP_NUM_THREADS message to log.warning;closed
pytorch/pytorch#63910 Add torch.distributed.is_torchelastic_launched() util method + make init_method=tcp:// compatible with torchelastic;closed
https://github.com/pytorch/pytorch/issues/62586;[v.1.9.1] Release Tracker ;open
pytorch/pytorch#62378: Fix elastic launch doc;closed
pytorch/pytorch#62139 option to skip GH validation for torch.hub;closed
pytorch/pytorch#62072 infinite loop in `_validate_not_a_forked_repo()`;closed
pytorch/pytorch#61394 mm not correctly report TORCH_CHECK failure issue;closed
https://github.com/pytorch/pytorch/pull/61294;[torch] Various improvements to `torch.distributed.launch` and `torch.distributed.run` (#60925);closed
pytorch/pytorch#60754 docs need an urgent serious update;closed
pytorch/pytorch#60752 `torch.distributed.launch` issues in 1.9.0;closed
https://github.com/pytorch/pytorch/issues/60717;torch.distributed.run future warning in 1.9.0;closed
https://github.com/pytorch/pytorch/issues/60716;torch.distributed runs under INFO log level in 1.9.0;closed
pytorch/pytorch#60318 Caffe2 thread-pool leak warning;closed
pytorch/pytorch#60059 warning on .names() access in max_pool2d and max_pool2d_backward;closed
pytorch/pytorch#58849 git stamp from release documentation version;open
pytorch/pytorch#57555 function for skipping module parameter / buffer initialization;closed
```
malfet added a commit to malfet/pytorch that referenced this pull request Sep 8, 2021
Summary:
Increase `page_idx` in the loop rather than outside of it
Break from the loop when receive empty response as it means there are no more items to fetch via pagination request

Also, add options to use provided github token (via `GITHUB_TOKEN` environment variable)

Fixes failure with "Rate Limit Exceeded" when doing something like `torch.hub.list("pytorch/test-infra:dsf")`

Fixes pytorch#61755

Pull Request resolved: pytorch#62072

Reviewed By: jbschlosser

Differential Revision: D29868539

Pulled By: malfet

fbshipit-source-id: 206082a0ba1208e9b15ff6c9c6cb71d2da74f1c3
malfet added a commit that referenced this pull request Sep 8, 2021
Summary:
Increase `page_idx` in the loop rather than outside of it
Break from the loop when receive empty response as it means there are no more items to fetch via pagination request

Also, add options to use provided github token (via `GITHUB_TOKEN` environment variable)

Fixes failure with "Rate Limit Exceeded" when doing something like `torch.hub.list("pytorch/test-infra:dsf")`

Fixes #61755

Pull Request resolved: #62072

Reviewed By: jbschlosser

Differential Revision: D29868539

Pulled By: malfet

fbshipit-source-id: 206082a0ba1208e9b15ff6c9c6cb71d2da74f1c3
malfet added a commit to malfet/pytorch that referenced this pull request Sep 8, 2021
Summary:
Split from pytorch#62072

Pull Request resolved: pytorch#62139

Reviewed By: mthrok

Differential Revision: D29891497

Pulled By: malfet

fbshipit-source-id: 5c0baf53a2acf8f95062bd001457e1f936011529
malfet added a commit that referenced this pull request Sep 8, 2021
Summary:
Split from #62072

Pull Request resolved: #62139

Reviewed By: mthrok

Differential Revision: D29891497

Pulled By: malfet

fbshipit-source-id: 5c0baf53a2acf8f95062bd001457e1f936011529
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.

HTTP Error 403 for torch.hub ResNet
4 participants