-
-
Notifications
You must be signed in to change notification settings - Fork 610
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
feat(plugins): add support for httpx
in B113
#1060
Conversation
requests.get('https://gmail.com') | ||
requests.get('https://gmail.com', timeout=None) | ||
requests.get('https://gmail.com', timeout=5) |
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.
Please don't remove these. They verify there's no regression when someone passes a timeout value
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.
They have not been removed but moved under "Okay" section below, since those cases did not trigger violations.
httpx.put('https://gmail.com', verify=True) | ||
httpx.put('https://gmail.com', verify=False) | ||
httpx.patch('https://gmail.com', verify=True) | ||
httpx.patch('https://gmail.com', verify=False) |
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.
Please just add a separate file rather than abuse this one
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.
Adding the check for missing timeouts on httpx
made this test file report medium violations in the test due to missing timeout values. I did that with the intention of keeping only the violations related to SSL verification disabled. Would you prefer to keep the file as is and bump the number of reported violations here instead?
For the missing timeouts, I can still create a separate test case and example file if you prefer though. I wanted to stay consistent with what has been done in #861, but having a separate test case would probably be preferrable.
Resolves #1059.