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.

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

fix(find-dependencies): http->https redirections weren't followed #431

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

nedbat
Copy link
Contributor

@nedbat nedbat commented Aug 17, 2023

When looking for the repos, we weren't following redirects from http to https, and so missed a number of repos.

See updated results in openedx/wg-build-test-release#238

@@ -119,6 +119,8 @@ def find_real_url(url: str) -> Optional[str]:
# I didn't know you could get 429 from https://github.com, but you can...
wait = int(resp.headers.get("Retry-After", 10))
time.sleep(wait + 1)
elif resp.status_code in {301, 302}:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think instead of doing this manually you should let requests handle it.

https://docs.python-requests.org/en/latest/user/quickstart/#redirection-and-history

This way you follow all the redirects in case things move multiple times.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm just realizing this, and wondering why the default of allow_redirects=True isn't applying here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, because I used HEAD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@nedbat nedbat force-pushed the nedbat/fix-find-dependencies branch from 4040308 to 8c44d96 Compare August 18, 2023 15:26
@nedbat nedbat merged commit 137eb5c into master Aug 21, 2023
2 checks passed
@nedbat nedbat deleted the nedbat/fix-find-dependencies branch August 21, 2023 21:00
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