-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
[pre-commit.ci] pre-commit autoupdate #225
Conversation
updates: - [github.com/Lucas-C/pre-commit-hooks.git: v1.5.4 → v1.5.5](https://github.com/Lucas-C/pre-commit-hooks.git/compare/v1.5.4...v1.5.5) - [github.com/python-jsonschema/check-jsonschema.git: 0.27.3 → 0.28.1](https://github.com/python-jsonschema/check-jsonschema.git/compare/0.27.3...0.28.1) - [github.com/adrienverge/yamllint.git: v1.33.0 → v1.35.1](https://github.com/adrienverge/yamllint.git/compare/v1.33.0...v1.35.1) - [github.com/PyCQA/flake8.git: 6.1.0 → 7.0.0](https://github.com/PyCQA/flake8.git/compare/6.1.0...7.0.0) - [github.com/PyCQA/flake8.git: 4.0.1 → 7.0.0](https://github.com/PyCQA/flake8.git/compare/4.0.1...7.0.0) - [github.com/PyCQA/pylint.git: v3.0.3 → v3.1.0](https://github.com/PyCQA/pylint.git/compare/v3.0.3...v3.1.0)
audience_url = f"https://{repository_domain}/_/oidc/audience" | ||
audience_resp = requests.get(audience_url) | ||
audience_url = f'https://{repository_domain}/_/oidc/audience' | ||
audience_resp = requests.get(audience_url, timeout=5) # S113 wants a timeout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@woodruffw I've set a timeout of 5 here and in the following invocation to please the linter. Should we set a lower value or split connect/read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that timeout seems fine to me (assuming that's seconds, which I think it is). IMO it could be lower but I suspect the 99.99% case will fall well inside this (both for normal and outage responses).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I'll let it be.
updates: