SeleniumManager python wrapper should check if architecture/platform combination is supported

[seidnerj]

Description

SeleniumManager's get_binary() now checks if architecture/platform combination is supported (and not only if the platform is supported).

Motivation and Context

Currently, when running selenium manager on unsupported architectures, best case scenario the user gets an OS level error message that's not necessarily clear, worst case scenario - the binary is completely missing and then the user gets a "Unable to obtain working Selenium Manager binary" message (for example, this happens on RP, since all RPG are set to install wheels from "piwheel" repo which seems to discard all executables that are not supported by RPs architectures).

Types of changes

• [] Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[CLAassistant]

All committers have signed the CLA.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3570209) 58.05% compared to head (cd3cada) 58.07%.
Report is 3 commits behind head on trunk.

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

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #13381      +/-   ##
==========================================
+ Coverage   58.05%   58.07%   +0.01%     
==========================================
  Files          88       88              
  Lines        5338     5340       +2     
  Branches      224      224              
==========================================
+ Hits         3099     3101       +2     
  Misses       2015     2015              
  Partials      224      224              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[titusfortner]

Aha, found the issue where I was originally discussing this, with this resolution: #11599 (comment)

I'm a little concerned about a strict allow list, but I guess if there are problems, we can update it again.

[seidnerj]

I'm not in-love with this solution but I think it's better than what we currently have, which I guess is also a kind of strict allow list as it is. We can implement a more comprehensive solution by checking the file format of the executable included or embedding this information during the build process but I don't think the ROI for that is worth it.

[seidnerj]

Just read everything, glad I could pitch in :)

[titusfortner]

Thanks!

[viniciusd]

Our code worked on previous versions on Docker running on Mac (hence Linux ARM) up to version 4.16. After this change, it won't initialize the browser anymore, so it looks like this was a breaking change

Is the SeleniumManager incompatible with linux/aarch64? How did things work on the previous versions? Should the WebDriver initializer fall back out of SeleniumManager if the architecture isn't allow-listed?

Edit

Allow-listing linux/aarch64 does the trick, i.e., adding ("linux", "x86_64"): "linux", to dirs enables my existing code to run again. Is this an acceptable solution? Should I make a PR with it?

[seidnerj]

Our code worked on previous versions on Docker running on Mac (hence Linux ARM) up to version 4.16. After this change, it won't initialize the browser anymore, so it looks like this was a breaking change

Is the SeleniumManager incompatible with linux/aarch64? How did things work on the previous versions? Should the WebDriver initializer fall back out of SeleniumManager if the architecture isn't allow-listed?

Edit

Allow-listing linux/aarch64 does the trick, i.e., adding ("linux", "x86_64"): "linux", to dirs enables my existing code to run again. Is this an acceptable solution? Should I make a PR with it?

You mean adding ("linux", "aarch64"): "linux", to dirs, right?

I was under the impression that binaries for aarch64 did not exist yet. @titusfortner?

[viniciusd]

You mean adding ("linux", "aarch64"): "linux", to dirs, right?

Yes, but it probably isn't the correct way of approaching it as it might not be supported by the Selenium manager.

I am looking into the code to understand what is happening. I have the driver installed in my PATH, I do not rely on the Selenium Manager, yet this check prevents Selenium from finding the driver as if relying on the Manager became mandatory.

I manage the binaries myself and this shouldn't be prevented. A failure on the Manager should still prompt a PATH search

[seidnerj]

I don't know in what order does Selenium resolve the binaries, if either first use Selenium Manager and fallback to binaries found in path or vice versa, but either way it doesn't sound like this has to do with this specific PR.

What you're saying really is that if Selenium Manager is not available no fallback to binaries potentially found in path occurs. That's a different and separate issue.

I am unfortunately not familiar with that code but you're of course welcome to fix this and create a PR for the maintainers' review (I am not a maintainer of Selenium).

[diemol]

https://www.selenium.dev/documentation/selenium_manager/#alternative-architectures

[seidnerj]

Thanks for this @diemol !

The relevant section: "If you are running Linux on arm64/aarch64, 32-bit architecture, or a Raspberry Pi, Selenium Manager will not work for you. The biggest issue for people is that they used to get custom-built drivers and put them on PATH and have them work. Now that Selenium Manager is responsible for locating drivers on PATH, this approach no longer works, and users need to use a Service class and set the location directly. There are a number of advantages to having Selenium Manager look for drivers on PATH instead of managing that logic in each of the bindings, so that’s currently a trade-off we are comfortable with."
@viniciusd

[viniciusd]

I don't know in what order does Selenium resolve the binaries, if either first use Selenium Manager and fallback to binaries found in path or vice versa, but either way it doesn't sound like this has to do with this specific PR.

What you're saying really is that if Selenium Manager is not available no fallback to binaries potentially found in path occurs. That's a different and separate issue.

I am unfortunately not familiar with that code but you're of course welcome to fix this and create a PR for the maintainers' review (I am not a maintainer of Selenium).

Thanks, and sorry for rushing into commenting on this PR as it was when Selenium starting breaking on my scenario.

Now I understand it wasn't caused by this change, it is a breaking change that was exposed on this PR because previously there was no architecture distinction (on 4.16.0, it just does an OS lookup).

And thanks @diemol for the reference

I will keep looking into it to see if I can propose an automatic fallback that the community would be comfortable with.

I find it particularly interesting/weird how 4.16.0 works fine even though it is resolving to the x86 selenium manager, which is supposed to be incompatible already

[seidnerj]

No problem 🙂

[viniciusd]

You two might find it interesting that the x86 manager is indeed running and returning the correct driver path on my arm64 (Docker on Mac):

# /usr/local/lib/python3.9/site-packages/selenium/webdriver/common/linux/selenium-manager --browser chrome --browser-path /usr/bin/chromium
INFO	Driver path: /usr/bin/chromedriver
INFO	Browser path: /usr/bin/chromium
# file /usr/local/lib/python3.9/site-packages/selenium/webdriver/common/linux/selenium-manager
/usr/local/lib/python3.9/site-packages/selenium/webdriver/common/linux/selenium-manager: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, stripped
# uname -a
Linux 6.6.12-linuxkit #1 aarch64 GNU/Linux

Probably some Debian magic happening behind the curtains, I couldn't find an explanation as I don't have Debian multiarch or even qemu installed. Well 🤷‍♂️

[seidnerj]

That's strange. I am not an expert on docker but my bet is that this has to do with the fact that at the end of the day you're really running on an arm processor (arm64 and aarch64 are just different names for the same arch).

Would love to know how this works though.