-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
feat(ssl-certificate): get ssl certificate support proxy #961
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(ssl-certificate): get ssl certificate support proxy #961
Conversation
|
here is test script |
aravindkarnam
left a comment
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.
@wakaka6 @unclecode I'm not very familiar with SSL certificate related concepts or PySocks library. So I'm not in a position to properly review this right away, but I'll do by next alpha release.
I just had a question w.r.t the dependancy, please check that out.
pyproject.toml
Outdated
| "faust-cchardet>=2.1.19", | ||
| "aiohttp>=3.11.11", | ||
| "humanize>=4.10.0", | ||
| "PySocks @ git+https://github.com/amirasaran/PySocks.git@3da955fd212ce02c3ab3bc166b5bfac3c91b4019" |
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.
@wakaka6 Could you explain why you're using a direct Git reference with a specific commit hash instead of the official PyPI package?
requirements.txt
Outdated
| cssselect>=1.2.0 | ||
| faust-cchardet>=2.1.19 | ||
| faust-cchardet>=2.1.19 | ||
| PySocks @ git+https://github.com/amirasaran/PySocks.git@3da955fd212ce02c3ab3bc166b5bfac3c91b4019 |
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.
Could you explain why you're using a direct Git reference with a specific commit hash instead of the official PyPI package?
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.
The official Pysocks has buggy(Anorov/PySocks#147) support for http proxies and the original author doesn't seem to have maintained it for a long time as well, so i need to use a specific version of Pysocks as a dependency.
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.
I read the mozilla documentation carefully and it seems that the http basic authentication should ignore case, so it's a problem with my proxy program, so we can just use the official version of pysocks.
|
Fixes: #778 - Tagging for visibility |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@aravindkarnam When can we get it to merge? I think I'm ready. |
|
this is fixed in the latest version, 0.7.6 |
Summary
Support proxy when getting ssl certificate
Same as #864 PR, but based on next branches.
List of files changed and why
ssl_ceritficate.py
ssl_ceritificate.to_playwright_format()str(ssl_ceritificate)proxy_config.py
self.usernameandself.password.async_crawler_strategy.py
How Has This Been Tested?
Checklist: