From 3e0a16ee52e0b77ae0ff8d793f597d212e0f474d Mon Sep 17 00:00:00 2001 From: cutz Date: Mon, 11 Jan 2021 14:57:34 -0600 Subject: [PATCH 1/5] depend on phantomjs_bin to provide the phantomjs binaries this project needs to run in isolation --- setup.py | 2 ++ src/nti/contentrendering/phantom.py | 27 ++++++++++++++------------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/setup.py b/setup.py index d9ad138..c850578 100755 --- a/setup.py +++ b/setup.py @@ -19,6 +19,7 @@ 'fudge', 'nti.testing', 'zope.testrunner', + 'nose >= 1.3.0' ] @@ -70,6 +71,7 @@ def _read(fname): 'nti.wref', 'Paste', 'PasteDeploy', + 'phantomjs-binary==2.1.3', 'PyPDF2', 'pyquery', 'pytz', diff --git a/src/nti/contentrendering/phantom.py b/src/nti/contentrendering/phantom.py index aaccdf9..1b360ff 100644 --- a/src/nti/contentrendering/phantom.py +++ b/src/nti/contentrendering/phantom.py @@ -9,11 +9,15 @@ from __future__ import absolute_import import os +import stat + import sys import subprocess import contextlib from six.moves import urllib_parse +from phantomjs_bin import executable_path as phantomjs_exe + try: from urllib.request import pathname2url except ImportError: @@ -27,6 +31,15 @@ logger = __import__('logging').getLogger(__name__) +# Make sure our phantomjs executable is, in fact, user executable +# https://github.com/jayjiahua/phantomjs-bin-pip/issues/1 +st = os.stat(phantomjs_exe) +if not st.st_mode & stat.S_IXUSR: + import warnings + warnings.warn('Phantomjs executable %s is not executable. Updating permissions' % (phantomjs_exe), + UserWarning, stacklevel=1) + os.chmod(phantomjs_exe, st.st_mode | stat.S_IXUSR) + def javascript_path(js_name): """ @@ -39,18 +52,6 @@ def javascript_path(js_name): return resource_filename(__name__, js_name) -if not os.getenv('DATASERVER_DIR_IS_BUILDOUT'): - # Buildout puts it first on the path - import warnings - try: - warnings.warn("Using whatever phantomjs is on the PATH; supported version 2.1.1; version found at %s is %s" - % (subprocess.check_output(['which', 'phantomjs']).strip(), - subprocess.check_output(['phantomjs', '-v']).strip()), - UserWarning, stacklevel=1) - except subprocess.CalledProcessError: - warnings.warn("Phantomjs not found on the PATH") - - class _PhantomProducedUnexpectedOutputError(subprocess.CalledProcessError): def __str__(self): @@ -105,7 +106,7 @@ def run_phantom_on_page(htmlFile, scriptName, args=(), key=_none_key, # over a socket as opposed to stdout/stderr? As of 1.6, I think this is the # recommended approach - process = ['phantomjs', scriptName, htmlFile] + process = [phantomjs_exe, scriptName, htmlFile] process.extend(args) __traceback_info__ = process logger.debug("Executing %s", process) From 72c6c380a1850a7452db5dd894e8896611accfc5 Mon Sep 17 00:00:00 2001 From: cutz Date: Mon, 11 Jan 2021 15:00:53 -0600 Subject: [PATCH 2/5] remove extra line --- src/nti/contentrendering/phantom.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nti/contentrendering/phantom.py b/src/nti/contentrendering/phantom.py index 1b360ff..a08bcf2 100644 --- a/src/nti/contentrendering/phantom.py +++ b/src/nti/contentrendering/phantom.py @@ -10,7 +10,6 @@ import os import stat - import sys import subprocess import contextlib From ff4bb33ddedf7adbc3467eef9fdf489a83c8382f Mon Sep 17 00:00:00 2001 From: cutz Date: Mon, 11 Jan 2021 15:21:52 -0600 Subject: [PATCH 3/5] Don't change perms at import time. do it when we invoke if we get a permission denied error. --- src/nti/contentrendering/phantom.py | 32 ++++++++++++++++++----------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/nti/contentrendering/phantom.py b/src/nti/contentrendering/phantom.py index a08bcf2..5b20901 100644 --- a/src/nti/contentrendering/phantom.py +++ b/src/nti/contentrendering/phantom.py @@ -8,6 +8,7 @@ from __future__ import print_function from __future__ import absolute_import +import errno import os import stat import sys @@ -30,16 +31,6 @@ logger = __import__('logging').getLogger(__name__) -# Make sure our phantomjs executable is, in fact, user executable -# https://github.com/jayjiahua/phantomjs-bin-pip/issues/1 -st = os.stat(phantomjs_exe) -if not st.st_mode & stat.S_IXUSR: - import warnings - warnings.warn('Phantomjs executable %s is not executable. Updating permissions' % (phantomjs_exe), - UserWarning, stacklevel=1) - os.chmod(phantomjs_exe, st.st_mode | stat.S_IXUSR) - - def javascript_path(js_name): """ :return: A path to a javascript resource of this package, suitable for passing to phantomjs. @@ -118,8 +109,25 @@ def run_phantom_on_page(htmlFile, scriptName, args=(), key=_none_key, stderr = open('/dev/null', 'wb') with _closing(stderr): - jsonStr = subprocess.check_output(process, stderr=stderr).strip() - __traceback_info__ += (jsonStr,) + try: + jsonStr = subprocess.check_output(process, stderr=stderr).strip() + __traceback_info__ += (jsonStr,) + except OSError as ex: + if ex.errno != errno.EACCES: + raise + + # Make sure our phantomjs executable is, in fact, user executable + # https://github.com/jayjiahua/phantomjs-bin-pip/issues/1 + import warnings + warnings.warn('Phantomjs executable %s is not executable. Updating permissions' % (phantomjs_exe), + UserWarning, stacklevel=1) + + mode = os.stat(process[0]).st_mode + os.chmod(process[0], mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + + # Try to execute again now that we have updated permissions + jsonStr = subprocess.check_output(process, stderr=stderr).strip() + __traceback_info__ += (jsonStr,) if expect_no_output: if jsonStr: From f70036488221bfaea6d8376a16ded2e264d60e63 Mon Sep 17 00:00:00 2001 From: cutz Date: Mon, 11 Jan 2021 15:29:24 -0600 Subject: [PATCH 4/5] cleanup imports. dry --- src/nti/contentrendering/phantom.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/nti/contentrendering/phantom.py b/src/nti/contentrendering/phantom.py index 5b20901..8b595d8 100644 --- a/src/nti/contentrendering/phantom.py +++ b/src/nti/contentrendering/phantom.py @@ -8,9 +8,7 @@ from __future__ import print_function from __future__ import absolute_import -import errno import os -import stat import sys import subprocess import contextlib @@ -111,8 +109,8 @@ def run_phantom_on_page(htmlFile, scriptName, args=(), key=_none_key, with _closing(stderr): try: jsonStr = subprocess.check_output(process, stderr=stderr).strip() - __traceback_info__ += (jsonStr,) except OSError as ex: + import errno if ex.errno != errno.EACCES: raise @@ -121,13 +119,14 @@ def run_phantom_on_page(htmlFile, scriptName, args=(), key=_none_key, import warnings warnings.warn('Phantomjs executable %s is not executable. Updating permissions' % (phantomjs_exe), UserWarning, stacklevel=1) - + + import stat mode = os.stat(process[0]).st_mode os.chmod(process[0], mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) # Try to execute again now that we have updated permissions jsonStr = subprocess.check_output(process, stderr=stderr).strip() - __traceback_info__ += (jsonStr,) + __traceback_info__ += (jsonStr,) if expect_no_output: if jsonStr: From 013764991ddc15d6764fa3807dc3543cb0199744 Mon Sep 17 00:00:00 2001 From: cutz Date: Mon, 11 Jan 2021 15:43:27 -0600 Subject: [PATCH 5/5] just use the logger now --- src/nti/contentrendering/phantom.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/nti/contentrendering/phantom.py b/src/nti/contentrendering/phantom.py index 8b595d8..96305ec 100644 --- a/src/nti/contentrendering/phantom.py +++ b/src/nti/contentrendering/phantom.py @@ -116,9 +116,7 @@ def run_phantom_on_page(htmlFile, scriptName, args=(), key=_none_key, # Make sure our phantomjs executable is, in fact, user executable # https://github.com/jayjiahua/phantomjs-bin-pip/issues/1 - import warnings - warnings.warn('Phantomjs executable %s is not executable. Updating permissions' % (phantomjs_exe), - UserWarning, stacklevel=1) + logger.warn('Phantomjs executable %s is not executable. Will attempt permissions update.', phantomjs_exe) import stat mode = os.stat(process[0]).st_mode