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

Allow configuration of the selenium docker repo #250

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

andrewnicols
Copy link
Contributor

For ARM chips, you may wish to use the seleniarm repo instead of selenium.

Please note that the 'seleniarm' repository contains officially sanctioned ARM images:

https://github.com/SeleniumHQ/docker-selenium#experimental-mult-arch-aarch64armhfamd64-images

For ARM chips, you may wish to use the `seleniarm` repo instead of
`selenium`.

Please note that the 'seleniarm' repository contains officially
sanctioned ARM images:

https://github.com/SeleniumHQ/docker-selenium#experimental-mult-arch-aarch64armhfamd64-images
@stronk7
Copy link
Member

stronk7 commented Mar 21, 2023

I was going to suggest to use, under Windows, the %PROCESSOR_ARCHITECTURE% to, similarly, look under Windows for a better default.

But them I've realised that there isn't Windows Docker Desktop for Arm64 yet (or I've not been able to find it).

So, could we just add a tiny comment after https://github.com/moodlehq/moodle-docker/pull/250/files#diff-31cc1d735f2a6a04693553db3ac6be87f39766a315913557db6a0921bf1bb40bR114 in order to say that, once there is Windows Docker Desktop for Arm we'll need to do something similar there (with %PROCESSOR_ARCHITECTURE%).

With that tiny detail, I'm happy processing this, thanks!

@stronk7
Copy link
Member

stronk7 commented Mar 21, 2023

Note this fixes #237

@stronk7
Copy link
Member

stronk7 commented Mar 21, 2023

And... one real problem... is that we default to MOODLE_DOCKER_BROWSER_TAG=3 and there aren't 3.x selearm images... so the docker compose command ends with error.

Maybe we should bump to 4 as default? Or only do it for ARM ? I've seen that, for core... they are not 100% ready / passing ... not sure ... there is moodlehq/moodle-ci-runner#87 related to that.

@stronk7
Copy link
Member

stronk7 commented Apr 8, 2023

Hi @andrewnicols ,

  1. I've added a tiny comment to the CMD as TODO, to point to the need of adding a condition if somedays there is a Windows Docker Desktop Arm64 available. Feel free to squash it.
  2. More important (see my previous comment), what to do with our current default of Sele3, when there aren't Sele3 images in selearm? Should we bump to Sele4 as default? At some point we certainly will have to, so...

Ciao :-)

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.

2 participants