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

Apple Silicon Support #76

Merged
merged 6 commits into from
Dec 4, 2023
Merged

Apple Silicon Support #76

merged 6 commits into from
Dec 4, 2023

Conversation

paulpall
Copy link
Contributor

@paulpall paulpall commented Dec 3, 2023

Changed the versions and suffixes to reflect the Apple Silicon drivers and updated downloader to work with newer Selenium.

@paulpall paulpall mentioned this pull request Dec 3, 2023
@soraxas
Copy link
Owner

soraxas commented Dec 4, 2023

Hi @paulpall thank you for the PR!

What is the reason for the commented kwargs?

            #self._driver = webdriver.Chrome(**kwargs)
            self._driver = webdriver.Chrome()

@soraxas
Copy link
Owner

soraxas commented Dec 4, 2023

@paulpall I've updated your PR on actually detecting arm processor, to avoid breaking older mac that uses intel cpu. Can you verify if the new changes still work on your computer?

@paulpall
Copy link
Contributor Author

paulpall commented Dec 4, 2023

Hi @paulpall thank you for the PR!

What is the reason for the commented kwargs?

            #self._driver = webdriver.Chrome(**kwargs)
            self._driver = webdriver.Chrome()

Honestly, the comments can be removed. The reason I didn't remove them was in case someone on a different platform ran into any issues, and because I didn't go through the codebase to see if kwargs were utilised elsewhere or if the firefox and chrome binary_downloader sections could be slimmed down.

@paulpall
Copy link
Contributor Author

paulpall commented Dec 4, 2023

@paulpall I've updated your PR on actually detecting arm processor, to avoid breaking older mac that uses intel cpu. Can you verify if the new changes still work on your computer?

Nice! Yup, that still seems to works fine for me.

@soraxas
Copy link
Owner

soraxas commented Dec 4, 2023

Hi @paulpall thank you for the PR!
What is the reason for the commented kwargs?

            #self._driver = webdriver.Chrome(**kwargs)
            self._driver = webdriver.Chrome()

Honestly, the comments can be removed. The reason I didn't remove them was in case someone on a different platform ran into any issues, and because I didn't go through the codebase to see if kwargs were utilised elsewhere or if the firefox and chrome binary_downloader sections could be slimmed down.

I don't quite understand what you meant; as in, was it that the original version with kwargs didn't work for you? Hence, you've commented them out?

@soraxas
Copy link
Owner

soraxas commented Dec 4, 2023

I'm asking that because they are used to set useragents on the browser, which was needed for the older (non-cloud) echo360 platform.

@paulpall
Copy link
Contributor Author

paulpall commented Dec 4, 2023

Hi @paulpall thank you for the PR!
What is the reason for the commented kwargs?

            #self._driver = webdriver.Chrome(**kwargs)
            self._driver = webdriver.Chrome()

Honestly, the comments can be removed. The reason I didn't remove them was in case someone on a different platform ran into any issues, and because I didn't go through the codebase to see if kwargs were utilised elsewhere or if the firefox and chrome binary_downloader sections could be slimmed down.

I don't quite understand what you meant; as in, was it that the original version with kwargs didn't work for you? Hence, you've commented them out?

Oh, yes it did not work, sorry. From what I could tell the executable_path was removed in Selenium 4.10.0 webdriver and is no longer required. However, it is possible to pass it as a Selenium service if required. Check this thread on Stack Overflow.

@soraxas
Copy link
Owner

soraxas commented Dec 4, 2023

In the requirements.txt there should be version specification on selenium:

selenium>=2.44.0,<4.1.4

@paulpall
Copy link
Contributor Author

paulpall commented Dec 4, 2023

Interesting, let me do a bit of digging...

@paulpall
Copy link
Contributor Author

paulpall commented Dec 4, 2023

In the requirements.txt there should be version specification on selenium:

selenium>=2.44.0,<4.1.4

It seems that the requirements aren't working properly when installing with pip install echo360. I uninstalled everything and cleared the cache and still got Selenium 4.15.2. Screenshot of the installation below:

Screenshot 2023-12-04 at 10 54 58

@soraxas
Copy link
Owner

soraxas commented Dec 4, 2023

In the requirements.txt there should be version specification on selenium:

selenium>=2.44.0,<4.1.4

It seems that the requirements aren't working properly when installing with pip install echo360. I uninstalled everything and cleared the cache and still got Selenium 4.15.2. Screenshot of the installation below:

Screenshot 2023-12-04 at 10 54 58

I see! Thanks for confirming!

@soraxas
Copy link
Owner

soraxas commented Dec 4, 2023

The latest push should auto handle newer selenium version. It would be great if you can test it on your end

@paulpall
Copy link
Contributor Author

paulpall commented Dec 4, 2023

The latest push should auto handle newer selenium version. It would be great if you can test it on your end

Unfortunately that's somehow trying to pass kwargs again, both with Firefox and Chrome. I will look into it a bit further to see why. Meanwhile I noticed that the package version at PyPI is out-of-date and that is probably the reason why it downloaded the latest Selenium for me.

@paulpall
Copy link
Contributor Author

paulpall commented Dec 4, 2023

The latest push should auto handle newer selenium version. It would be great if you can test it on your end

Unfortunately that's somehow trying to pass kwargs again, both with Firefox and Chrome. I will look into it a bit further to see why. Meanwhile I noticed that the package version at PyPI is out-of-date and that is probably the reason why it downloaded the latest Selenium for me.

Never mind, I cleaned everything and ran it again, works great!

@soraxas
Copy link
Owner

soraxas commented Dec 4, 2023

Awesome! Thanks @paulpall for confirming! I'll merge the new changes and will also push to PyPI afterwards.

@soraxas soraxas merged commit 5f79ed8 into soraxas:master Dec 4, 2023
1 check failed
This was linked to issues Dec 4, 2023
@soraxas soraxas mentioned this pull request Dec 4, 2023
@paulpall paulpall deleted the apple_silicon_support branch December 4, 2023 18:57
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.

chromedriver for M1 mac? Outdated chromedriver
2 participants