-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
docs: update docker container and celery worker chrome installation #29315
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Malcolm Thanks for this PR and sorry it took so long for you to get a review! It looks great to me but I haven't set up Chrome for my reporting and this isn't something I know much about. I've put this to a few people for review and hopefully someone can take a look. And if no one is able to weigh in, I'd say let's err on the side of merging this - I'll put a calendar reminder for myself to circle back here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we switch to a solution that is deterministic?
Sorry I didn't see that we hard-code the CHROME_VERSION
Disregard my previous review/comment, is there not an easier/cleaner way to install this!? I didn't like the way we did it before, and this seems like a good iteration based on this approach, but would there be a better way? |
Hey, looking at |
Maybe we don't need to run:
This command will install these packages:
Additionally, we can obtain a version of Chrome using a command, so we don't need to fix one:
|
Can we look into using the |
SUMMARY
The existing documentation has the user download the latest release of Chrome and use ChromeDriver v102. This combination doesn't function, providing the user with an error saying the Chrome version is unsupported from ChromeDriver during alert/report generation.
This update pulls both the version of Chrome and ChromeDriver from the Chrome Labs repository which is centrally maintained and allows for both the Chrome and ChromeDriver version to stay in lock step.
Additionally the Superset base image version and Chrome versions can be set as build-args during image creation.
TESTING INSTRUCTIONS
docs/docs/configuration/alerts-reports.mdx
with a customised tagx-superset-image
indocker-compose.yml
to point to your customised tagADDITIONAL INFORMATION