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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[馃悰 Bug]: [Python] WebDriver constructor must not assume that BaseOptions instance has _ignore_local_proxy property #10755

Closed
mykola-mokhnach opened this issue Jun 9, 2022 · 9 comments 路 Fixed by #13926
Closed

[馃悰 Bug]: [Python] WebDriver constructor must not assume that BaseOptions instance has _ignore_local_proxy property #10755

mykola-mokhnach opened this issue Jun 9, 2022 · 9 comments 路 Fixed by #13926
Assignees
@titusfortner
Labels
C-py I-defect
Milestone
4.21

Comments

@mykola-mokhnach
Copy link

What happened?

I'm talking about

selenium/py/selenium/webdriver/remote/webdriver.py

Line 256 in 27ff586

_ignore_local_proxy = options._ignore_local_proxy

The typing info says options: Union[BaseOptions, List[BaseOptions]] = None, while _ignore_local_proxy property is only defined for ArgOptions class. This means any code that inherits Options from BaseOptions rather than from ArgOptions would fail with missing attribute error (which is also happening for the Appium Python client)

How can we reproduce the issue?

Pass any custom class inherited from BaseOptions rather than from ArgOptions to the WebDriver constructor

Relevant log output

if options:
                capabilities = options.to_capabilities()
>               _ignore_local_proxy = options._ignore_local_proxy
E               AttributeError: 'UiAutomator2Options' object has no attribute '_ignore_local_proxy'


### Operating System

macOS

### Selenium version

Python 3.9

### What are the browser(s) and version(s) where you see this issue?

Appium

### What are the browser driver(s) and version(s) where you see this issue?

Any

### Are you using Selenium Grid?

_No response_
@github-actions
Copy link

github-actions bot commented Jun 9, 2022

@mykola-mokhnach, thank you for creating this issue. We will troubleshoot it as soon as we can.

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

@mykola-mokhnach
Copy link
Author

I assume such issue could be easily caught with mypy automatically

@symonk
Copy link
Member

symonk commented Jun 9, 2022 •
edited

It could but we have a lot of work to get that going, types at the moment are quite frankly very messy, any PRs introducing improved types would be greatly appreciated. tox -e mypy exists but its extremely broken and not enforced in CI at all, it's a mammoth task to get everything typed etc

@symonk symonk added the C-py label Jun 9, 2022
@github-actions
Copy link

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the I-stale Applied to issues that become stale, and eventually closed. label Mar 17, 2023
@symonk symonk removed the I-stale Applied to issues that become stale, and eventually closed. label Mar 17, 2023
@github-actions GitHub Actions
Copy link

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the I-stale Applied to issues that become stale, and eventually closed. label Dec 22, 2023
@titusfortner
Copy link
Member

I think the fix for this should have been just moving it from ArgOptions to BaseOptions?
At any rate, I finally have a PR to at least deprecate this parameter entirely: #13286

@mykola-mokhnach
Copy link
Author

Thanks @titusfortner for taking care of it.
This also means we'd need to adjust Appium Python client after the next Selenium release.
FYI @KazuCocoa

@github-actions github-actions bot removed the I-stale Applied to issues that become stale, and eventually closed. label Dec 23, 2023
@titusfortner titusfortner self-assigned this Dec 29, 2023
@titusfortner titusfortner modified the milestones: 4.17, 4.18 Jan 18, 2024
@diemol diemol modified the milestones: 4.18, 4.19 Feb 16, 2024
@diemol diemol modified the milestones: 4.19, 4.20 Mar 25, 2024
@diemol diemol modified the milestones: 4.20, 4.21 Apr 17, 2024
@diemol diemol mentioned this issue May 10, 2024
Merged
8 tasks
@diemol
Copy link
Member

diemol commented May 10, 2024

@mykola-mokhnach would #13926 be enough while we merge @titusfortner's PR?

@diemol
Copy link
Member

diemol commented May 13, 2024

I will merge it and if we need to fix something else, we iterate on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@titusfortner titusfortner

Labels
C-py I-defect
Projects
None yet
Milestone
4.21
Development

Successfully merging a pull request may close this issue.

Moving ignore_local_proxy_environment_variables to BaseOptions SeleniumHQ/selenium
4 participants
@titusfortner @diemol @mykola-mokhnach @symonk