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

Cause kaleido to explicitly fail if no chromium: #224

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

ayjayt
Copy link
Collaborator

@ayjayt ayjayt commented Nov 15, 2024

Before we were letting choreographer handle it.

Before we were letting choreographer handle it.
@ayjayt ayjayt requested a review from gvwilson November 15, 2024 19:56
@ayjayt
Copy link
Collaborator Author

ayjayt commented Nov 15, 2024

Hey Greg, this error will be more clean for users that they need to have chrome installed!

@ayjayt ayjayt linked an issue Nov 15, 2024 that may be closed by this pull request
@ayjayt
Copy link
Collaborator Author

ayjayt commented Nov 15, 2024

Not sure if it solves #223 but it may

@@ -59,6 +59,8 @@ def __init__(self, plotlyjs=None, mathjax=None, topojson=None, mapbox_access_tok
'kaleido_scopes.js'
)
path = os.environ.get("BROWSER_PATH", which_browser())
if not path:
raise RuntimeError("Kaleido now requires that chrome/chromium is installed separately. Kaleido will try to detect it automatically, but the environmental error \"BROWSER_PATH\" can also be set")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise RuntimeError("Kaleido now requires that chrome/chromium is installed separately. Kaleido will try to detect it automatically, but the environmental error \"BROWSER_PATH\" can also be set")
raise RuntimeError("Kaleido now requires that chrome/chromium is installed separately. Kaleido will try to detect it automatically, but the environment variable \"BROWSER_PATH\" can also be set")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow okay this is correct why did I miss this? thank you

@gvwilson gvwilson merged commit 6abcd02 into master Nov 18, 2024
4 checks passed
@ayjayt ayjayt linked an issue Nov 18, 2024 that may be closed by this pull request
@ayjayt ayjayt mentioned this pull request Nov 18, 2024
@ayjayt ayjayt deleted the andrew/raise_if_no_path branch November 18, 2024 18:59
@pdmkdz
Copy link

pdmkdz commented Nov 18, 2024

Not sure how to circumvent this new requirement in a serverless compute on Azure Databricks...
It seems chromium cannot be accessed in those conditions and will always end up in error if trying to save plotly plots as static images.

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.

Upgrade kaleido 0.2.1 -> 0.4.1 is causing errors in CI Kaleido 0.4 image export fails
4 participants