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

[py] Remove deprecated code in driver classes #12030

Merged
merged 5 commits into from
Jun 1, 2023
Merged

[py] Remove deprecated code in driver classes #12030

merged 5 commits into from
Jun 1, 2023

Conversation

titusfortner
Copy link
Member

@titusfortner titusfortner commented May 12, 2023

Description

  • get_remote_connection() is kind of a clever hack to be able to use browser specific methods in remote webdriver class. I would love if we can get a mixin or something to better support these. Regardless, what is there currently does not work for Chrome or Edge because the browser_name of the ChromiumRemoteConnection is None.
  • Removed the keep_alive deprecation notice. It doesn't belong in Service class, so it is better off where it is still. Probably what we want is to have a "ClientConfig" class like Java where user can specify keep_alive, proxy, timeouts (and number of retries?). It would also let us better handle _ignore_local_proxy which is currently getting set in Options. Anyway, future PR.
  • The driver class constructors have defaults so I removed some of the "if None" type protections that I don't think are needed. I'm pretty sure this logic covers everything.
  • Removed w3c option warnings. The driver can error if it has a problem with anything w3c compliant that gets passed along.

Motivation and Context

It's really difficult to see what is going on in Python code with deprecations everywhere
All of this has been deprecated since 4.0 or earlier.

I'm honestly not sure when to set things to self and when not to, and when to reference things as self, so this code can probably be cleaned up even more.

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2023

Codecov Report

Patch coverage: 63.51% and project coverage change: -0.25 ⚠️

Comparison is base (c1e47d3) 55.93% compared to head (5d790b2) 55.69%.

❗ Current head 5d790b2 differs from pull request most recent head 5e7a7bf. Consider uploading reports for the commit 5e7a7bf to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #12030      +/-   ##
==========================================
- Coverage   55.93%   55.69%   -0.25%     
==========================================
  Files          86       86              
  Lines        5746     5421     -325     
  Branches      233      223      -10     
==========================================
- Hits         3214     3019     -195     
+ Misses       2299     2179     -120     
+ Partials      233      223      -10     
Impacted Files Coverage Δ
py/selenium/webdriver/chromium/options.py 75.30% <ø> (+1.15%) ⬆️
py/selenium/webdriver/remote/webdriver.py 40.22% <0.00%> (-1.90%) ⬇️
py/selenium/webdriver/remote/webelement.py 35.46% <ø> (-0.13%) ⬇️
py/selenium/webdriver/safari/webdriver.py 47.36% <50.00%> (-9.78%) ⬇️
py/selenium/webdriver/safari/service.py 74.35% <57.14%> (-3.77%) ⬇️
py/selenium/webdriver/firefox/webdriver.py 63.63% <72.72%> (+10.22%) ⬆️
py/selenium/webdriver/ie/webdriver.py 84.61% <72.72%> (+6.69%) ⬆️
py/selenium/webdriver/edge/webdriver.py 80.00% <77.77%> (+2.50%) ⬆️
py/selenium/webdriver/chrome/webdriver.py 95.00% <100.00%> (+22.08%) ⬆️
py/selenium/webdriver/chromium/webdriver.py 45.09% <100.00%> (-9.85%) ⬇️
... and 1 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@titusfortner titusfortner force-pushed the py_deps branch 2 times, most recently from cc7cf9e to 16c16a6 Compare May 12, 2023 18:10
@titusfortner titusfortner marked this pull request as draft May 12, 2023 18:59
@titusfortner titusfortner force-pushed the py_deps branch 6 times, most recently from 3734122 to 42a19ba Compare May 14, 2023 23:31
@titusfortner titusfortner marked this pull request as ready for review May 15, 2023 00:24
@AutomatedTester
Copy link
Member

Initial review and looks ok

@titusfortner titusfortner added this to the 4.11 milestone May 18, 2023
@titusfortner titusfortner force-pushed the py_deps branch 2 times, most recently from 9a89264 to 1fce40c Compare May 27, 2023 03:13
@titusfortner titusfortner modified the milestones: 4.11, 4.10 May 27, 2023
@titusfortner
Copy link
Member Author

I tried a few things to get the lint test passing on CI, and nothing is working. Not sure how to proceed since things are passing just fine for me locally.

@diemol diemol merged commit 9f5801c into trunk Jun 1, 2023
14 checks passed
@diemol diemol deleted the py_deps branch June 1, 2023 09:41
evansd added a commit to ebmdatalab/openprescribing that referenced this pull request Dec 14, 2023
This will allow us to update Selenium.

I determined the changes needed here by looking at the PR which removed
the deprecated code and working backwards:
SeleniumHQ/selenium#12030
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants