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

Update electron version to 2.0.8 to address segmentation fault #125

Closed
wants to merge 5 commits into from

Conversation

jonmmease
Copy link
Contributor

This PR updates the electron version from 1.8.4 to 2.0.8 to address the segmentation fault error discussed in #124.

I verified on MacOS that the segmentation fault no longer occurs when the NODE_OPTIONS environment variable is set.

My local tests all passed without an issue, now we'll see what the CI tests do.

@etpinard
Copy link
Contributor

Great.

We'll need to test this on stage, to make sure the new electron behaves well enough for our image-server before merging this.

@scjody what are some of the tests you ran last spring?

@scjody
Copy link
Contributor

scjody commented Aug 29, 2018

@etpinard I did my testing from the jody-imageserver-test GCE instance in the "Plotly Cloud Production" GCP project (stage is also in that project, so anything that needs to access the imageserver directly must be in there too). Feel free to use this instance - I'll keep it running for now.

I used a modified version of the plotly.js testing tools, which can be found on the imageserver-testing-hacks branch, along with these datasets. Note that the tests can make calls via a streambed instance or directly to an imageserver; you want direct mode.

@etpinard
Copy link
Contributor

Thanks @scjody !

Unfortunately @jonmmease , I don't think I'll have time to get to this until at least Friday if not next Tuesday.

If not too busy, perhaps @tarzzz could help us out?

@jonmmease
Copy link
Contributor Author

I totally understand @etpinard, this sounds a bit involved. I'll focus on the rest of the plotly.py 3.2 release items and them make the call on whether to block on this or not. Thanks!

@jonmmease
Copy link
Contributor Author

@etpinard I'm in the process of adding a workaround to plotly.py that will simply unset the NODE_OPTIONS environment variable before calling orca. See plotly/plotly.py#1140.

This takes care of the segmentation fault problem when orca is used from a plotly.py session that was launched with the NODE_OPTIONS environment variable set. Now that I know that this workaround is possible, I don't think we need to block plotly.py 3.2.0 on this PR 🙂

@etpinard
Copy link
Contributor

etpinard commented Sep 4, 2018

I don't think we need to block plotly.py 3.2.0 on this PR

Is this still the case @jonmmease ?

@jonmmease
Copy link
Contributor Author

Yes. My workaround is working well. Thanks for following up!

@etpinard
Copy link
Contributor

etpinard commented Sep 4, 2018

Yes. My workaround is working well. Thanks for following up!

Fantastic. I'll differ testing Electron 2.0 to after the plotly.js v1.41.0 release (in hopefully about ~1 week).

@tonkolviktor
Copy link

Hello,

+1

On raspberry pi 3 b+ with version 1.8.4 of electron and latest node [1] i get the following error [2].
The reason is that the server is never started, because "did-finish-load" event is never emitted in src/app/server/index.js. (I do not know why or why the page is not loaded!?)

After applying the changes manually in this PR it works for me, would be nice to have it in the master ;)

(BTW first i tried with electron 5.X, the server could start but the plot generation was just hanging without any apparent reason)

[1]
pi@raspberrypi:~ $ node -v
v10.15.3
pi@raspberrypi:~ $ npm -v
6.9.0

[2]
ValueError:
For some reason plotly.py was unable to communicate with the
local orca server process, even though the server process seems to be running.

@antoinerg antoinerg mentioned this pull request Jul 3, 2019
@antoinerg antoinerg mentioned this pull request Nov 12, 2019
5 tasks
@antoinerg
Copy link
Collaborator

Superseded and closed by #266

@antoinerg antoinerg closed this Jan 8, 2020
@antoinerg antoinerg deleted the electron_2.0.8 branch January 8, 2020 20:07
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.

5 participants