From 8a1a4e8437e360c0c91fcf8c4d450175a11640c4 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 28 Aug 2018 07:44:52 -0400 Subject: [PATCH 1/8] WIP to import error reporting when something goes wrong validating orca Display the error message or invalid help result at the end of the message --- plotly/io/_orca.py | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/plotly/io/_orca.py b/plotly/io/_orca.py index 91c5ae34ac4..5cbe71d8068 100644 --- a/plotly/io/_orca.py +++ b/plotly/io/_orca.py @@ -887,7 +887,8 @@ def validate_executable(): >>> plotly.io.orca.config.save() If you're still having trouble, feel free to ask for help on the forums at -https://community.plot.ly/c/api/python""" +https://community.plot.ly/c/api/python +""" # Try to find an executable # ------------------------- @@ -923,32 +924,49 @@ def validate_executable(): try: help_result = subprocess.check_output([executable, '--help']) - except subprocess.CalledProcessError: - raise ValueError(invalid_executable_msg) + except subprocess.CalledProcessError as err: + + raise ValueError(invalid_executable_msg + """ +Here is the error that was returned by the command + $ {executable} --help + +{err_msg} +""".format(executable=executable, err_msg=str(err))) if not help_result: - raise ValueError(invalid_executable_msg) + raise ValueError(invalid_executable_msg + """ +The error encountered is that no output was returned by the command + $ {executable} --help +""".format(executable=executable)) if ("Plotly's image-exporting utilities" not in help_result.decode('utf-8')): - raise ValueError(invalid_executable_msg) + raise ValueError(invalid_executable_msg + """ +The error encountered is that unexpected output was returned by the command + $ {executable} --help + +{help_result} +""".format(executable=executable, help_result=help_result)) # Get orca version # ---------------- try: orca_version = subprocess.check_output([executable, '--version']) - except subprocess.CalledProcessError: - raise ValueError(""" + except subprocess.CalledProcessError as err: + raise ValueError(invalid_executable_msg + """ An error occurred while trying to get the version of the orca executable. Here is the command that plotly.py ran to request the version: $ {executable} --version -""".format(executable=executable)) + +This command returned the following error: - if not orca_version: - raise ValueError(""" -No version was reported by the orca executable. +{err_msg} +""".format(executable=executable, err_msg=str(err))) + if not orca_version: + raise ValueError(invalid_executable_msg + """ +The error encountered is that no version was reported by the orca executable. Here is the command that plotly.py ran to request the version: $ {executable} --version From d206dd08ad02ee2566b717c4b9db1907ead8f777 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 28 Aug 2018 08:04:48 -0400 Subject: [PATCH 2/8] Don't raise exception if killing child process fails on exit --- plotly/io/_orca.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/plotly/io/_orca.py b/plotly/io/_orca.py index 5cbe71d8068..422677c31c5 100644 --- a/plotly/io/_orca.py +++ b/plotly/io/_orca.py @@ -1030,18 +1030,28 @@ def shutdown_server(): # process. This prevents any zombie processes from being # left over, and it saves us from needing to write # OS-specific process management code here. + parent = psutil.Process(orca_state['proc'].pid) for child in parent.children(recursive=True): - child.terminate() - - # Kill parent process - orca_state['proc'].terminate() - - # Retrieve standard out and standard error to avoid warnings - output, err = orca_state['proc'].communicate() - - # Wait for the process to shutdown - child_status = orca_state['proc'].wait() + try: + child.terminate() + except: + # We tried, move on + pass + + try: + # Kill parent process + orca_state['proc'].terminate() + + # Retrieve standard out and standard error to avoid + # warnings + output, err = orca_state['proc'].communicate() + + # Wait for the process to shutdown + child_status = orca_state['proc'].wait() + except: + # We tried, move on + pass # Update our internal process management state orca_state['proc'] = None From 2af8e8bf0518217a1285dc10692b254ffb8b2ad0 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 28 Aug 2018 08:38:30 -0400 Subject: [PATCH 3/8] Use Popen to --help and --version checks and add error text to our exception --- plotly/io/_orca.py | 46 +++++++++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/plotly/io/_orca.py b/plotly/io/_orca.py index 422677c31c5..065ea1789c9 100644 --- a/plotly/io/_orca.py +++ b/plotly/io/_orca.py @@ -922,21 +922,29 @@ def validate_executable(): executable=executable, instructions=install_location_instructions) - try: - help_result = subprocess.check_output([executable, '--help']) - except subprocess.CalledProcessError as err: + # ### Run with Popen so we get access to stdout and stderr + p = subprocess.Popen( + [executable, '--help'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + help_result, help_error = p.communicate() + if p.returncode != 0: raise ValueError(invalid_executable_msg + """ Here is the error that was returned by the command $ {executable} --help +[Return code: {returncode}] {err_msg} -""".format(executable=executable, err_msg=str(err))) + """.format(executable=executable, + err_msg=help_error, + returncode=p.returncode)) if not help_result: raise ValueError(invalid_executable_msg + """ The error encountered is that no output was returned by the command - $ {executable} --help + $ {executable} --help """.format(executable=executable)) if ("Plotly's image-exporting utilities" not in @@ -950,21 +958,29 @@ def validate_executable(): # Get orca version # ---------------- - try: - orca_version = subprocess.check_output([executable, '--version']) - except subprocess.CalledProcessError as err: + # ### Run with Popen so we get access to stdout and stderr + p = subprocess.Popen( + [executable, '--version'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + version_result, version_error = p.communicate() + + if p.returncode != 0: raise ValueError(invalid_executable_msg + """ An error occurred while trying to get the version of the orca executable. -Here is the command that plotly.py ran to request the version: - +Here is the command that plotly.py ran to request the version $ {executable} --version - + This command returned the following error: +[Return code: {returncode}] {err_msg} -""".format(executable=executable, err_msg=str(err))) + """.format(executable=executable, + err_msg=version_error, + returncode=p.returncode)) - if not orca_version: + if not version_result: raise ValueError(invalid_executable_msg + """ The error encountered is that no version was reported by the orca executable. Here is the command that plotly.py ran to request the version: @@ -972,10 +988,10 @@ def validate_executable(): $ {executable} --version """.format(executable=executable)) else: - orca_version = orca_version.decode() + version_result = version_result.decode() status._props['executable'] = executable - status._props['version'] = orca_version.strip() + status._props['version'] = version_result.strip() status._props['state'] = 'validated' From 5d0494a7832ee1e3255b48f4799b0d2d936d8c3f Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 28 Aug 2018 08:40:30 -0400 Subject: [PATCH 4/8] decode error message --- plotly/io/_orca.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plotly/io/_orca.py b/plotly/io/_orca.py index 065ea1789c9..5106bf3a9b2 100644 --- a/plotly/io/_orca.py +++ b/plotly/io/_orca.py @@ -938,7 +938,7 @@ def validate_executable(): [Return code: {returncode}] {err_msg} """.format(executable=executable, - err_msg=help_error, + err_msg=help_error.decode('utf-8'), returncode=p.returncode)) if not help_result: @@ -977,7 +977,7 @@ def validate_executable(): [Return code: {returncode}] {err_msg} """.format(executable=executable, - err_msg=version_error, + err_msg=version_error.decode('utf-8'), returncode=p.returncode)) if not version_result: From 3e40d19ac2379a76e62db891e9607dcbabf6faa8 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 28 Aug 2018 08:46:55 -0400 Subject: [PATCH 5/8] Explain that error details are at the end of the message --- plotly/io/_orca.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plotly/io/_orca.py b/plotly/io/_orca.py index 5106bf3a9b2..596b079121e 100644 --- a/plotly/io/_orca.py +++ b/plotly/io/_orca.py @@ -915,8 +915,9 @@ def validate_executable(): # --------------------------------------------------- invalid_executable_msg = """ The orca executable is required in order to export figures as static images, -but the executable that was found at '{executable}' does not seem to be a -valid plotly orca executable. +but the executable that was found at '{executable}' +does not seem to be a valid plotly orca executable. Please refer to the end of +this message for details on what went wrong. {instructions}""".format( executable=executable, From 82c62a6ae3fd5026734fa99db93d5c7b80f31a88 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 28 Aug 2018 09:15:55 -0400 Subject: [PATCH 6/8] Add X11 display check error message. --- plotly/io/_orca.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/plotly/io/_orca.py b/plotly/io/_orca.py index 596b079121e..1f64e743f9a 100644 --- a/plotly/io/_orca.py +++ b/plotly/io/_orca.py @@ -932,15 +932,25 @@ def validate_executable(): help_result, help_error = p.communicate() if p.returncode != 0: - raise ValueError(invalid_executable_msg + """ + err_msg = invalid_executable_msg + """ Here is the error that was returned by the command $ {executable} --help [Return code: {returncode}] {err_msg} - """.format(executable=executable, - err_msg=help_error.decode('utf-8'), - returncode=p.returncode)) +""".format(executable=executable, + err_msg=help_error.decode('utf-8'), + returncode=p.returncode) + + # Check for Linux without X installed. + if (sys.platform.startswith('linux') and + not os.environ.get('DISPLAY')): + + err_msg += """ +Note: when used on Linux orca requires an X11 display server, but none was +detected. Please install x11, or configure your system with Xvfb. +""" + raise ValueError(err_msg) if not help_result: raise ValueError(invalid_executable_msg + """ From 7dbcb05136b4665b911b158df7a6f64ce7d9144e Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 28 Aug 2018 09:19:26 -0400 Subject: [PATCH 7/8] Adjust line spacing --- plotly/io/_orca.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plotly/io/_orca.py b/plotly/io/_orca.py index 1f64e743f9a..6b9ad617854 100644 --- a/plotly/io/_orca.py +++ b/plotly/io/_orca.py @@ -946,7 +946,7 @@ def validate_executable(): if (sys.platform.startswith('linux') and not os.environ.get('DISPLAY')): - err_msg += """ + err_msg += """\ Note: when used on Linux orca requires an X11 display server, but none was detected. Please install x11, or configure your system with Xvfb. """ From e1b20566a291668923a7416c08bdf6cf1552ee0a Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Tue, 28 Aug 2018 10:30:22 -0400 Subject: [PATCH 8/8] Adjust line spacing --- plotly/io/_orca.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plotly/io/_orca.py b/plotly/io/_orca.py index 6b9ad617854..d2f163d8d01 100644 --- a/plotly/io/_orca.py +++ b/plotly/io/_orca.py @@ -947,8 +947,10 @@ def validate_executable(): not os.environ.get('DISPLAY')): err_msg += """\ -Note: when used on Linux orca requires an X11 display server, but none was -detected. Please install x11, or configure your system with Xvfb. +Note: When used on Linux, orca requires an X11 display server, but none was +detected. Please install X11, or configure your system with Xvfb. See +the orca README (https://github.com/plotly/orca) for instructions on using +orca with Xvfb. """ raise ValueError(err_msg)