Skip to content

Running image export bug fixed #307

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

Merged
merged 11 commits into from
Jul 5, 2022
Merged

Conversation

WhiteBlackGoose
Copy link
Contributor

@WhiteBlackGoose WhiteBlackGoose commented Jul 1, 2022

The issue is that when rendering an image it fails to start chromium browser on Linux

@WhiteBlackGoose WhiteBlackGoose marked this pull request as draft July 1, 2022 12:57
@WhiteBlackGoose
Copy link
Contributor Author

Why 😢 . It produces the same image, but different string, varying in character 1002. The CI is on ubuntu 22.04, just like my machine...

@kMutagene
Copy link
Collaborator

Why 😢 . It produces the same image, but different string, varying in character 1002. The CI is on ubuntu 22.04, just like my machine...

Yeah that's the exact reason why i gave up on those tests and just started to skip them, the exact string does not seem to be deterministic (although it's kinda strange)

@kMutagene
Copy link
Collaborator

Regarding the --no-sandbox thing - There is already a mutable variable exposed where you should be able to set this:

let mutable launchOptions = LaunchOptions()
so you should be able to just set the flag for your environment

Comment on lines 16 to 17
// https://github.com/hardkoded/puppeteer-sharp/issues/1211
launchOptions.Args <- [| "--no-sandbox" |]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should it be linux-only?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i do not think we need to hard-code this at all. I have used this lib on linux without setting this. as we expose the launch settings, this can be set by the user if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's imo too obscure. I spent some time investigating it, and we leave it as it, not working for some linux users?

Copy link
Collaborator

Choose a reason for hiding this comment

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

is this on the contrary guaranteed to work everywhere else where it wasn't needed before? if so, i guess theres no harm

Copy link
Contributor Author

@WhiteBlackGoose WhiteBlackGoose Jul 4, 2022

Choose a reason for hiding this comment

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

Can't say for sure. Windows and Linux tests pass though. Is there some reason why we don't have those for MacOS btw?

(also I can set this option only for Linux platform, this is no problem)

@WhiteBlackGoose WhiteBlackGoose marked this pull request as ready for review July 1, 2022 14:11
@WhiteBlackGoose WhiteBlackGoose requested a review from kMutagene July 4, 2022 13:50
@kMutagene kMutagene merged commit a176304 into plotly:dev Jul 5, 2022
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.

2 participants