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

Allows parametrizing requests' session #225

Closed
wants to merge 16 commits into from

Conversation

miguelius
Copy link

@miguelius miguelius commented Sep 22, 2021

This version handles all calls to requests to a function which returns session object customizable. In this case we can disable verify ssl. This could fix #219.

Usage:

    driver_path = GeckoDriverManager().dont_verify_ssl().install()
    ff = webdriver.Firefox(executable_path=driver_path)
    ff.get("http://automation-remarks.com")
    ff.quit()

@pep8speaks
Copy link

pep8speaks commented Sep 22, 2021

Hello @miguelius! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-10-18 09:53:10 UTC

@miguelius
Copy link
Author

Hi @SergeyPirogov! Is it possible to run the workflow? If it works, how far is it of being included in a release?

Thank you very much!

Cheers!

@miguelius
Copy link
Author

Fixed conflicts!

@aleksandr-kotlyar
Copy link
Collaborator

Also: Closes #226

aleksandr-kotlyar and others added 5 commits October 17, 2021 21:16
…ix a huge typo in IEDriver constructor (appeared in 3.5.0 version); bump version to 3.5.1 (SergeyPirogov#248)
…version and OS type (SergeyPirogov#249)

- Feature: finding EdgeDriver version for MAC & LINUX depends on MSEdge browser version and OS type (Closes SergeyPirogov#243, SergeyPirogov#242)
- Fix: Add rights to execute edgedriver binary on linux.
- Test Coverage: More tests for EdgeDriver & testing on LINUX/MAC in CI.
- delete requirements.txt
@miguelius
Copy link
Author

Conflict resolved!

@aleksandr-kotlyar
Copy link
Collaborator

@miguelius i am not sure that using globals is necessary. May be it's better to realize through variable like

driver =  EdgeChromiumDriverManager(ssl_verify=False).install()

@aleksandr-kotlyar
Copy link
Collaborator

And it is also #211 - with same request but with a bit larger functionality. We can take ssl_verify from this as example. And implement only this for now, then think about proxies.

@miguelius
Copy link
Author

miguelius commented Oct 18, 2021

Using an argument in the constructor was my first call, still I had problems with function download_file, which calls directly to requests.

While it's implemented using a global hidden variable, this means it only could be used by session() function and reset by new_session(), It's more like a singleton withouth the boiler plate. Nobody uses the _session variable. And it was the shortest path with minimum impact in the code without changing any method signatures.

#211 isn't enough, because you still have the problem with ssl verification when you are using https proxy.

I'll propose you an alternative in another pr. I believe requests session should be the parameter, and this should be used by both the webdrivermanager an utility functions in order to work properly.

Thank you for your time!
** edited: tried to clarify a paragraph, corrected negation and any use, and added the thank you I forgot in the first version of the comment 😄

@miguelius miguelius closed this Oct 19, 2021
@miguelius
Copy link
Author

better implementation available!

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.

failed to install geckodriver
3 participants