Commit e29ba689 authored by Douglas's avatar Douglas Committed by Ivan Tyagov

erp5_kernel: render errors of wrong environment.define and undefine calls in Jupyter

@Tyagov, please review.

Errors currently captured:

- environment.define call 1st argument is not a function and/or 2nd argument is not a string
- environment.define call argument is not a string
- environment.undefine trying to remove a function that is not in the environment

Proper tests were added.

/reviewed-on nexedi/erp5!144
parent 5a7536ce
...@@ -195,7 +195,16 @@ def Base_runJupyterCode(self, jupyter_code, old_notebook_context): ...@@ -195,7 +195,16 @@ def Base_runJupyterCode(self, jupyter_code, old_notebook_context):
import_fixer = ImportFixer() import_fixer = ImportFixer()
environment_collector = EnvironmentParser() environment_collector = EnvironmentParser()
ast_node = import_fixer.visit(ast_node) ast_node = import_fixer.visit(ast_node)
# The collector also raises errors when environment.define and undefine
# calls are made incorrectly, so we need to capture them to propagate
# to Jupyter for rendering.
try:
ast_node = environment_collector.visit(ast_node) ast_node = environment_collector.visit(ast_node)
except (EnvironmentDefinitionError, EnvironmentUndefineError) as e:
transaction.abort()
return getErrorMessageForException(self, e, notebook_context)
# Get the node list from the parsed tree # Get the node list from the parsed tree
nodelist = ast_node.body nodelist = ast_node.body
...@@ -277,7 +286,17 @@ def Base_runJupyterCode(self, jupyter_code, old_notebook_context): ...@@ -277,7 +286,17 @@ def Base_runJupyterCode(self, jupyter_code, old_notebook_context):
break break
if not found: if not found:
transaction.abort() transaction.abort()
raise Exception("Trying to remove non existing function/variable from environment: '%s'\nEnvironment: %s" % (func_alias, str(notebook_context['setup']))) result = {
'result_string': "EnvironmentUndefineError: Trying to remove non existing function/variable from environment: '%s'\n" % func_alias,
'notebook_context': notebook_context,
'status': 'ok',
'mime_type': 'text/plain',
'evalue': None,
'ename': None,
'traceback': None,
}
return result
# Removing all the setup functions if user call environment.clearAll() # Removing all the setup functions if user call environment.clearAll()
if environment_collector.clearAll(): if environment_collector.clearAll():
...@@ -402,6 +421,14 @@ def Base_runJupyterCode(self, jupyter_code, old_notebook_context): ...@@ -402,6 +421,14 @@ def Base_runJupyterCode(self, jupyter_code, old_notebook_context):
return result return result
class EnvironmentUndefineError(TypeError):
pass
class EnvironmentDefinitionError(TypeError):
pass
def canSerialize(obj): def canSerialize(obj):
result = False result = False
...@@ -503,11 +530,20 @@ class EnvironmentParser(ast.NodeTransformer): ...@@ -503,11 +530,20 @@ class EnvironmentParser(ast.NodeTransformer):
name = attribute.id name = attribute.id
if name == 'environment' and function.attr == 'define' and not value.keywords: if name == 'environment' and function.attr == 'define' and not value.keywords:
if not len(value.args) == 2: if not len(value.args) == 2:
message = ( raise EnvironmentDefinitionError('environment.define calls receive 2 arguments')
'Not enough arguments for environment definition. Function '
'name and alias are required.' self._ensureType(
obj=value.args[0],
klass=ast.Name,
error_message='Type mismatch. environment.define receives a function as first argument.'
)
self._ensureType(
obj=value.args[1],
klass=ast.Str,
error_message='Type mismatch. environment.define receives a string as second argument.'
) )
raise Exception(message)
func_name = value.args[0].id func_name = value.args[0].id
func_alias = value.args[1].s func_alias = value.args[1].s
function_node = self.function_dict[func_name] function_node = self.function_dict[func_name]
...@@ -532,6 +568,13 @@ class EnvironmentParser(ast.NodeTransformer): ...@@ -532,6 +568,13 @@ class EnvironmentParser(ast.NodeTransformer):
arg_value = node_value_dict[type(arg_value_node)](arg_value_node) arg_value = node_value_dict[type(arg_value_node)](arg_value_node)
self.environment_var_dict[arg_name] = arg_value self.environment_var_dict[arg_name] = arg_value
elif name == 'environment' and function.attr == 'undefine': elif name == 'environment' and function.attr == 'undefine':
self._ensureType(
obj=value.args[0],
klass=ast.Str,
call_type='undefine',
error_message='Type mismatch. environment.undefine receives only a string as argument.'
)
func_alias = value.args[0].s func_alias = value.args[0].s
self.environment_remove_list.append(func_alias) self.environment_remove_list.append(func_alias)
elif name == 'environment' and function.attr == 'clearAll': elif name == 'environment' and function.attr == 'clearAll':
...@@ -540,6 +583,14 @@ class EnvironmentParser(ast.NodeTransformer): ...@@ -540,6 +583,14 @@ class EnvironmentParser(ast.NodeTransformer):
self.show_environment_setup = True self.show_environment_setup = True
return node return node
def _ensureType(self, obj=None, klass=None, error_message=None, call_type='define'):
if not isinstance(obj, klass):
if call_type == 'define':
error_class = EnvironmentDefinitionError
elif call_type == 'undefine':
error_class = EnvironmentUndefineError
raise error_class(error_message)
def clearAll(self): def clearAll(self):
return self.environment_clear_all return self.environment_clear_all
...@@ -912,4 +963,3 @@ def erp5PivotTableUI(self, df): ...@@ -912,4 +963,3 @@ def erp5PivotTableUI(self, df):
iframe_host = self.REQUEST['HTTP_X_FORWARDED_HOST'].split(',')[0] iframe_host = self.REQUEST['HTTP_X_FORWARDED_HOST'].split(',')[0]
url = "https://%s/erp5/Base_displayPivotTableFrame?key=%s" % (iframe_host, key) url = "https://%s/erp5/Base_displayPivotTableFrame?key=%s" % (iframe_host, key)
return IFrame(src=url, width='100%', height='500') return IFrame(src=url, width='100%', height='500')
...@@ -614,6 +614,69 @@ print random.randint(1,1) ...@@ -614,6 +614,69 @@ print random.randint(1,1)
self.assertEquals(result['status'], 'ok') self.assertEquals(result['status'], 'ok')
self.assertEquals(result['code_result'].strip(), '1') self.assertEquals(result['code_result'].strip(), '1')
def testEnvorinmentUndefineErrors(self):
"""
Tests if environment.undefine wrong usage errors are correctly captured
and rendered in Jupyter.
"""
self.login('dev_user')
undefine_not_found = 'environment.undefine("foobar")'
reference = 'Test.Notebook.EnvironmentObject.Errors.Undefine'
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=undefine_not_found
)
self.tic()
error_substring = 'EnvironmentUndefineError: Trying to remove non existing'
self.assertTrue(error_substring in result)
not_string_code = 'def foobar(): pass\nenvironment.undefine(foobar)'
reference = 'Test.Notebook.EnvironmentObject.Errors.Undefine'
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=not_string_code
)
self.tic()
error_substring = 'EnvironmentUndefineError: Type mismatch.'
self.assertTrue(error_substring in result)
def testEnvironmentDefineErrrors(self):
"""
Tests if environment.define wrong usage errors are correctly captured
and rendered in Jupyter.
"""
self.login('dev_user')
first_arg_type_code = "environment.define('foobar', 'foobar')"
reference = 'Test.Notebook.EnvironmentObject.Errors.Define'
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=first_arg_type_code
)
self.tic()
error_substring = 'EnvironmentDefinitionError: Type mismatch'
self.assertTrue(error_substring in result)
self.assertTrue('first argument' in result)
second_arg_type_code = 'def couscous(): pass\nenvironment.define(couscous, 123)'
reference = 'Test.Notebook.EnvironmentObject.Errors.Define'
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=second_arg_type_code
)
self.tic()
error_substring = 'EnvironmentDefinitionError: Type mismatch'
self.assertTrue(error_substring in result)
self.assertTrue('second argument' in result)
def testPivotTableJsIntegration(self): def testPivotTableJsIntegration(self):
''' '''
This test ensures the PivotTableJs user interface is correctly integrated This test ensures the PivotTableJs user interface is correctly integrated
......
...@@ -44,7 +44,7 @@ ...@@ -44,7 +44,7 @@
<key> <string>text_content_warning_message</string> </key> <key> <string>text_content_warning_message</string> </key>
<value> <value>
<tuple> <tuple>
<string>W:457, 4: Unused variable \'notebook\' (unused-variable)</string> <string>W:697, 4: Unused variable \'notebook\' (unused-variable)</string>
</tuple> </tuple>
</value> </value>
</item> </item>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment