-
Notifications
You must be signed in to change notification settings - Fork 55
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
Allow passing of custom arguments to urllib Request object #34
Conversation
Codecov Report
@@ Coverage Diff @@
## master #34 +/- ##
=========================================
- Coverage 72.54% 71.05% -1.5%
=========================================
Files 5 5
Lines 641 646 +5
=========================================
- Hits 465 459 -6
- Misses 176 187 +11
Continue to review full report at Codecov.
|
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.
Thank you for the pull request. This is generally great and thank you for your time.
I have few minor cosmetic changed I would like you to change before merging in.
Please let me know if you have the time to fix it.
Thanks
pySmartDL/pySmartDL.py
Outdated
@@ -75,14 +77,21 @@ class SmartDL: | |||
* If no path is provided, `%TEMP%/pySmartDL/` will be used. | |||
''' | |||
|
|||
def __init__(self, urls, dest=None, progress_bar=True, fix_urls=True, threads=5, timeout=5, logger=None, connect_default_logger=False): | |||
def __init__(self, urls, dest=None, progress_bar=True, fix_urls=True, threads=5, timeout=5, logger=None, connect_default_logger=False, requestArgs=None): |
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.
- this is user-facing API and therefore requestArgs should become request_args
- I feel like it should come after "fix_urls" or something, and not at the end of the constructor.
- The place must be in sync with the docs
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.
1 and 3 have been fixed. As for 2, I put it at the end as to not break any code that currrently uses positional arguments. I can change it if you'd like though.
Thanks |
- fixes over MR #34 - get_json method for SmartDL - better logging syntax - added tests - change test files host from hetzner.de to ovh
- fixes over MR #34 - get_json method for SmartDL - better logging syntax - added tests - change test files host from hetzner.de to ovh
- fixes over MR #34 - get_json method for SmartDL - better logging syntax - added tests - change test files host from hetzner.de to ovh
- fixes over MR #34 - get_json method for SmartDL - better logging syntax - added tests - updated user agents - change test files host from hetzner.de to ovh
This will allow for more granular control of headers, proxies, and anything else that urllib provides an option for without having to implement the functionality in pySmartDL. Closes #33