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

Clear and restore problematic NODE environment variables before calling orca #1378

Merged
merged 2 commits into from
Dec 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 47 additions & 20 deletions plotly/io/_orca.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import threading
import warnings
from copy import copy
from contextlib import contextmanager

import requests
import retrying
Expand Down Expand Up @@ -836,6 +837,36 @@ def __repr__(self):
del OrcaStatus


@contextmanager
def orca_env():
"""
Context manager to clear and restore environment variables that are
problematic for orca to function properly
NODE_OPTIONS: When this variable is set, orca <v1.2 will have a
segmentation fault due to an electron bug.
See: https://github.com/electron/electron/issues/12695
ELECTRON_RUN_AS_NODE: When this environment variable is set the call
to orca is transformed into a call to nodejs.
See https://github.com/plotly/orca/issues/149#issuecomment-443506732
"""
clear_env_vars = ['NODE_OPTIONS', 'ELECTRON_RUN_AS_NODE']
orig_env_vars = {}

try:
# Clear and save
orig_env_vars.update({
var: os.environ.pop(var)
for var in clear_env_vars
if var in os.environ})
yield
finally:
# Restore
for var, val in orig_env_vars.items():
os.environ[var] = val


# Public orca server interaction functions
# ----------------------------------------
def validate_executable():
Expand Down Expand Up @@ -918,13 +949,6 @@ def validate_executable():
formatted_path=formatted_path,
instructions=install_location_instructions))

# Clear NODE_OPTIONS environment variable
# ---------------------------------------
# When this variable is set, orca <v1.2 will have a segmentation fault
# due to an electron bug.
# See: https://github.com/electron/electron/issues/12695
os.environ.pop('NODE_OPTIONS', None)

# Run executable with --help and see if it's our orca
# ---------------------------------------------------
invalid_executable_msg = """
Expand All @@ -938,12 +962,13 @@ def validate_executable():
instructions=install_location_instructions)

# ### Run with Popen so we get access to stdout and stderr
p = subprocess.Popen(
[executable, '--help'],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
with orca_env():
p = subprocess.Popen(
[executable, '--help'],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)

help_result, help_error = p.communicate()
help_result, help_error = p.communicate()

if p.returncode != 0:
err_msg = invalid_executable_msg + """
Expand Down Expand Up @@ -986,12 +1011,13 @@ def validate_executable():
# Get orca version
# ----------------
# ### Run with Popen so we get access to stdout and stderr
p = subprocess.Popen(
[executable, '--version'],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
with orca_env():
p = subprocess.Popen(
[executable, '--version'],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)

version_result, version_error = p.communicate()
version_result, version_error = p.communicate()

if p.returncode != 0:
raise ValueError(invalid_executable_msg + """
Expand Down Expand Up @@ -1171,8 +1197,9 @@ def ensure_server():
# Create subprocess that launches the orca server on the
# specified port.
DEVNULL = open(os.devnull, 'wb')
orca_state['proc'] = subprocess.Popen(cmd_list,
stdout=DEVNULL)
with orca_env():
orca_state['proc'] = subprocess.Popen(cmd_list,
stdout=DEVNULL)

# Update orca.status so the user has an accurate view
# of the state of the orca server
Expand All @@ -1191,7 +1218,7 @@ def ensure_server():
orca_state['shutdown_timer'] = t


@retrying.retry(wait_random_min=5, wait_random_max=10, stop_max_delay=8000)
@retrying.retry(wait_random_min=5, wait_random_max=10, stop_max_delay=30000)
def request_image_with_retrying(**kwargs):
"""
Helper method to perform an image request to a running orca server process
Expand Down
4 changes: 4 additions & 0 deletions plotly/tests/test_orca/test_orca_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
# --------
@pytest.fixture()
def setup():
# Set problematic environment variables
os.environ['NODE_OPTIONS'] = '--max-old-space-size=4096'
os.environ['ELECTRON_RUN_AS_NODE'] = '1'

# Reset orca state
pio.orca.reset_status()
pio.orca.config.restore_defaults()
Expand Down
37 changes: 37 additions & 0 deletions plotly/tests/test_orca/test_to_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,3 +283,40 @@ def test_topojson_fig_to_image(topofig, format):
def test_latex_fig_to_image(latexfig, format):
img_bytes = pio.to_image(latexfig, format=format, width=700, height=500)
assert_image_bytes(img_bytes, 'latexfig.' + format)


# Environmnet variables
# ---------------------
def test_problematic_environment_variables(fig1, format):
pio.orca.config.restore_defaults(reset_server=True)

os.environ['NODE_OPTIONS'] = '--max-old-space-size=4096'
os.environ['ELECTRON_RUN_AS_NODE'] = '1'

# Do image export
img_bytes = pio.to_image(fig1, format=format, width=700, height=500)
assert_image_bytes(img_bytes, 'fig1.' + format)

# Check that environment variables were restored
assert os.environ['NODE_OPTIONS'] == '--max-old-space-size=4096'
assert os.environ['ELECTRON_RUN_AS_NODE'] == '1'


# Invalid figure json
# -------------------
def test_invalid_figure_json():
# Do image export
bad_fig = {'foo': 'bar'}
with pytest.raises(ValueError) as err:
pio.to_image(bad_fig, format='png')

assert "Invalid value of type" in str(err.value)

with pytest.raises(ValueError) as err:
pio.to_image(bad_fig, format='png', validate=False)

assert ('The image request was rejected by the orca conversion utility'
in str(err.value))

assert ('400: invalid or malformed request syntax'
in str(err.value))