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

ProxyAdded proxy option + proxy sanity checks #277

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

holzkohlengrill
Copy link

Added proxy option as a command line argument.

Added checks to make sure the proxy URL is valid.

Copy link
Owner

@lorenzodifuccia lorenzodifuccia left a comment

Choose a reason for hiding this comment

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

Hi @holzkohlengrill, thank you for the PR.
Could you please see the comment on the changed file?

safaribooks.py Outdated
if USE_PROXY: # DEBUG
self.session.proxies = PROXIES
self.session.verify = False
if args_parsed.proxies:
Copy link
Owner

Choose a reason for hiding this comment

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

Use self.args instead of global args_parsed

Copy link
Author

Choose a reason for hiding this comment

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

@lorenzodifuccia please check. I removed now completely the global arguments and packed it into some functions to avoid errors like this and make it more readable.

@holzkohlengrill
Copy link
Author

@lorenzodifuccia anything open or can we merge?

safaribooks.py Outdated Show resolved Hide resolved
safaribooks.py Outdated Show resolved Hide resolved
@lorenzodifuccia
Copy link
Owner

Hi @holzkohlengrill, I thought I sent the request change, sorry 🤦🏻‍♂️

@lorenzodifuccia
Copy link
Owner

The Proxy args and code it's ok.
But, why you changed also the way the program handle the arguments?
Could you rebase the code from the master branch and add only the proxy's related code?

@holzkohlengrill
Copy link
Author

I changed it because it was very messy and error prone due to a mix of global and local arguments, etc.

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.

None yet

2 participants