erp5_data_notebook: moves variables and setup storage from ActiveProcess to...

erp5_data_notebook: moves variables and setup storage from ActiveProcess to `notebook_context` property

Notebook's variables and setup were being stored in an ActiveProcess, which is
not needed anymore, because now everything related to user context can be
safely stored in the ZODB.
They are stored in a `notebook_context` property of the Data Notebook
itself. Code and tests were updated properly. The old `process` property was removed.

All the references to *_variable_dict were renamed to  *_notebook_context, documentation
and tests were updated. Related objects like scripts and external methods were renamed
too.

To store objects in the `notebook_context` property we do 2 different tests: the first
to check if the object can be serialized by `ZODB.serialize.ObjectWriter`. If the first
test fails we test if the object can be serialized by cPickle. For the second test we
need to dump & load the object to be completely sure that it can be correctly loaded
later.

The function called by the Base_runJupyterCode external method was renamed from
Base_compileJupyterCode to Base_runJupyterCode be more consistent and avoid confusion.

All errors while running setup functions and now properly propagated to the user
interface in Jupyter and code execution is aborted.

bt5/erp5_data_notebook/ExtensionTemplateItem/portal_components/extension.erp5.JupyterCompile.py

 # -*- coding: utf-8 -*-
 
 from cStringIO import StringIO
-from Products.ERP5Type.Globals import  PersistentMapping
 from erp5.portal_type import Image
 from types import ModuleType
+from ZODB.serialize import ObjectWriter
 
 import sys
 import traceback
@@ -12,6 +12,7 @@ import base64
 import cPickle
 
 import transaction
+import Acquisition
 import astor
 
 from matplotlib.figure import Figure
@@ -19,22 +20,30 @@ from IPython.core.display import DisplayObject
 from IPython.lib.display import IFrame
 
 
-def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
+def Base_runJupyterCode(self, jupyter_code, old_notebook_context):
   """
-    Function to execute jupyter code and update the local_varibale dictionary.
+    Function to execute jupyter code and update the context dictionary.
     Code execution depends on 'interactivity', a.k.a , if the ast.node object has
-    ast.Expr instance(valid for expressions) or not.
+    ast.Expr instance (valid for expressions) or not.
     
-    old_local_variable_dict should contain both variables dict and modules imports.
-    Here, imports dict is key, value pair of modules and their name in sys.path,
-    executed separately everytime before execution of jupyter_code to populate
-    sys modules beforehand.
-
-    For example :
-    old_local_variable_dict = {
-                                'imports': {'numpy': 'np', 'sys': 'sys'},
-                                'variables': {'np.split': <function split at 0x7f4e6eb48b90>}
-                                }
+    old_notebook_context should contain both variables dict and setup functions.
+    Here, setup dict is {key: value} pair of setup function names and another dict,
+    which contains the function's alias and code, as string. These functions
+    should be executed before `jupyter_code` to properly create the required
+    environment.
+
+    For example:
+    old_notebook_context =  {
+      'setup': {
+        'numpy setup': {
+          'func_name': 'numpy_setup_function',
+          'code': ...
+        }
+      },
+      'variables': {
+        'my_variable': 1
+      }
+    }
 
     The behaviour would be similar to that of jupyter notebook:-
     ( https://github.com/ipython/ipython/blob/master/IPython/core/interactiveshell.py#L2954 )
@@ -65,26 +74,17 @@ def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
   # Saving the initial globals dict so as to compare it after code execution
   globals_dict = globals()
   result_string = ''
-
-  # XXX: The focus is on 'ok' status only, we're letting errors to be raised on
-  # erp5 for now, so as not to hinder the transactions while catching them.
-  # TODO: This can be refactored by using client side error handling instead of
-  # catching errors on server/erp5.
-  #
-  local_variable_dict = copy.deepcopy(old_local_variable_dict)
+  notebook_context = old_notebook_context
 
   # Execute only if jupyter_code is not empty
-  #
   if jupyter_code:
     # Create ast parse tree
-    #
     try:
       ast_node = ast.parse(jupyter_code)
     except Exception as e:
       # It's not necessary to abort the current transaction here 'cause the 
       # user's code wasn't executed at all yet.
-      #
-      return getErrorMessageForException(self, e, local_variable_dict)
+      return getErrorMessageForException(self, e, notebook_context)
     
     # Fixing "normal" imports and detecting environment object usage
     import_fixer = ImportFixer()
@@ -93,15 +93,12 @@ def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
     ast_node = environment_collector.visit(ast_node)
     
     # Get the node list from the parsed tree
-    #
     nodelist = ast_node.body
 
     # Handle case for empty nodelist(in case of comments as jupyter_code)
-    #
     if nodelist:
       # If the last node is instance of ast.Expr, set its interactivity as 'last'
       # This would be the case if the last node is expression
-      #
       if isinstance(nodelist[-1], ast.Expr):
         interactivity = "last"
       else:
@@ -147,23 +144,21 @@ def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
         '_volatile_variable_list': []
       }
       user_context.update(inject_variable_dict)
-      user_context.update(local_variable_dict['variables'])
+      user_context.update(notebook_context['variables'])
       
       # Getting the environment setup defined in the current code cell
-      #
       current_setup_dict = environment_collector.getEnvironmentSetupDict()
       current_var_dict = environment_collector.getEnvironmentVarDict()
 
       # Removing old setup from the setup functions
-      #
       removed_setup_message_list = []
       for func_alias in environment_collector.getEnvironmentRemoveList():
         found = False
-        for key, data in local_variable_dict['setup'].items():
+        for key, data in notebook_context['setup'].items():
           if key == func_alias:
             found = True
             func_name = data['func_name']
-            del local_variable_dict['setup'][func_alias]
+            del notebook_context['setup'][func_alias]
             try:
               del user_context[func_alias]
             except KeyError:
@@ -176,17 +171,17 @@ def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
             removed_setup_message_list.append(removed_setup_message)
             break
         if not found:
-          raise Exception("Trying to remove non existing function/variable from environment: '%s'\nEnvironment: %s" % (func_alias, str(local_variable_dict['setup'])))
+          transaction.abort()
+          raise Exception("Trying to remove non existing function/variable from environment: '%s'\nEnvironment: %s" % (func_alias, str(notebook_context['setup'])))
       
       # Removing all the setup functions if user call environment.clearAll()
       if environment_collector.clearAll():
-        keys = local_variable_dict['setup'].keys()
+        keys = notebook_context ['setup'].keys()
         for key in keys:
-          del local_variable_dict['setup'][key]
+          del notebook_context['setup'][key]
       
       # Running all the setup functions that we got
-      #
-      for key, value in local_variable_dict['setup'].iteritems():
+      for key, value in notebook_context['setup'].iteritems():
         try:
           code = compile(value['code'], '<string>', 'exec')
           exec(code, user_context, user_context)
@@ -214,31 +209,25 @@ def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
           "_volatile_variable_list += _result.keys()\n"
           "del %s, _result\n"
         ) % (data['code'], func_name, func_name)
-        local_variable_dict['setup'][data['alias']] = {
+        notebook_context['setup'][data['alias']] = {
           "func_name": func_name,
           "code": setup_string
         }
 
       # Iterating over envinronment.define calls captured by the environment collector
       # that are simple variables and saving them in the setup.
-      #
       for variable, value, in current_var_dict.iteritems():
         setup_string = "%s = %s\n" % (variable, repr(value))
-        local_variable_dict['setup'][variable] = {
+        notebook_context['setup'][variable] = {
           'func_name': variable,
           'code': setup_string
         }
         user_context['_volatile_variable_list'] += variable
         
       if environment_collector.showEnvironmentSetup():
-        result_string += "%s\n" % str(local_variable_dict['setup'])
-        # environment_list = []
-        # for func_alias, data in local_variable_dict['setup'].iteritems():
-        #   environment_list.append([data['func_name'], func_alias])
-        # result_string += "%s\n" % environment_list
+        result_string += "%s\n" % str(notebook_context['setup'])
 
       # Execute the nodes with 'exec' mode
-      #
       for node in to_run_exec:
         mod = ast.Module([node])
         code = compile(mod, '<string>', "exec")
@@ -249,12 +238,11 @@ def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
           # are not added if an exception occurs.
           #
           # TODO: store which notebook line generated which exception.
-          #
           transaction.abort()
-          return getErrorMessageForException(self, e, local_variable_dict)
+          # Clear the portal cache from previous transaction
+          return getErrorMessageForException(self, e, notebook_context)
 
       # Execute the interactive nodes with 'single' mode
-      #
       for node in to_run_interactive:
         mod = ast.Interactive([node])
         try:
@@ -265,9 +253,9 @@ def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
           # are not added if an exception occurs.
           #
           # TODO: store which notebook line generated which exception.
-          #
           transaction.abort()
-          return getErrorMessageForException(self, e, local_variable_dict)
+          # Clear the portal cache from previous transaction
+          return getErrorMessageForException(self, e, notebook_context)
 
       sys.stdout = old_stdout
       mime_type = display_data['mime_type'] or mime_type
@@ -276,32 +264,53 @@ def Base_compileJupyterCode(self, jupyter_code, old_local_variable_dict):
     # Checking in the user context what variables are pickleable and we can store
     # safely. Everything that is not pickleable shall not be stored and the user
     # needs to be warned about it.
-    #
     volatile_variable_list = current_setup_dict.keys() + inject_variable_dict.keys() + user_context['_volatile_variable_list']
     del user_context['_volatile_variable_list']
     for key, val in user_context.items():
       if not key in globals_dict.keys() and not isinstance(val, ModuleType) and not key in volatile_variable_list:
+        can_store = False
+        
+        # Try to check if we can serialize the object in a way which it can be
+        # stored properly in the ZODB 
         try:
-          import pickle
-          pickle.dumps(val)
-          local_variable_dict['variables'][key] = val
+          # Need to unwrap the variable, otherwise we get a TypeError, because
+          # objects cannot be pickled while inside an acquisition wrapper.
+          ObjectWriter(val).serialize(Acquisition.aq_base(val))
+          can_store = True
+        # If cannot serialize object with ZODB.serialize, try with cPickle
         except:
+          try:
+            # Only a dump of the object is not enough. Dumping and trying to
+            # load it will properly raise errors in all possible situations, 
+            # for example: if the user defines a dict with an object of a class 
+            # that he created the dump will stil work, but the load will fail. 
+            cPickle.loads(cPickle.dumps(val))
+            can_store = True
+          except:
+            can_store = False
+          
+        if can_store:
+          notebook_context['variables'][key] = val
+        else:
           del user_context[key]
-          result_string += ("Cannot pickle the variable named %s whose value is %s, "
-                            "thus it will not be stored in the context. "
-                            "You should move it's definition to a function and " 
-                            "use the environment object to load it.\n") % (key, val)
+          result_string += (
+            "Cannot pickle the variable named %s whose value is %s, "
+            "thus it will not be stored in the context. "
+            "You should move it's definition to a function and " 
+            "use the environment object to load it.\n"
+          ) % (key, val)
+        # if isinstance(val, InstanceType):
+        #   can_pickle = False
     
     # Deleting from the variable storage the keys that are not in the user 
-    # context anymore (i.e., variables that are deleted by the user)
-    #
-    for key in local_variable_dict['variables'].keys():
+    # context anymore (i.e., variables that are deleted by the user).
+    for key in notebook_context['variables'].keys():
       if not key in user_context:
-        del local_variable_dict['variables'][key]
+        del notebook_context['variables'][key]
 
   result = {
     'result_string': result_string,
-    'local_variable_dict': local_variable_dict,
+    'notebook_context': notebook_context,
     'status': status,
     'mime_type': mime_type,
     'evalue': evalue,
@@ -531,7 +540,7 @@ def renderAsHtml(self, renderable_object):
   # At this point the stack should be, from top to the bottom:
   #
   #   5. ExternalMethod Patch call
-  #   4. Base_compileJupyterCode frame (where we want to change variable)
+  #   4. Base_runJupyterCode frame (where we want to change variable)
   #   3. exec call to run the user's code
   #   2. ExternalMethod Patch call through `context.Base_renderAsHtml` in the notebook
   #   1. renderAsHtml frame (where the function is)
@@ -545,10 +554,10 @@ def renderAsHtml(self, renderable_object):
   compile_jupyter_locals['result'].write(result)
   compile_jupyter_locals['display_data']['mime_type'] = 'text/html'
 
-def getErrorMessageForException(self, exception, local_variable_dict):
+def getErrorMessageForException(self, exception, notebook_context):
   '''
     getErrorMessageForException receives an Expcetion object and a context for
-    code execution (local_variable_dict) and will return a dict as Jupyter
+    code execution (notebook_context) and will return a dict as Jupyter
     requires for error rendering.
   '''
   etype, value, tb = sys.exc_info()
@@ -556,38 +565,18 @@ def getErrorMessageForException(self, exception, local_variable_dict):
   return {
     'status': 'error',
     'result_string': None,
-    'local_variable_dict': local_variable_dict,
+    'notebook_context': notebook_context,
     'mime_type': 'text/plain',
     'evalue': str(value),
     'ename': exception.__class__.__name__,
     'traceback': traceback_text
   }
 
-def AddNewLocalVariableDict(self):
-  """
-  Function to add a new Local Variable for a Data Notebook
-  """
-  new_dict = PersistentMapping()
-  variable_dict = PersistentMapping()
-  module_dict = PersistentMapping()
-  setup_dict = PersistentMapping()
-  new_dict['variables'] = variable_dict
-  new_dict['imports'] = module_dict
-  new_dict['setup'] = setup_dict 
-  return new_dict
-
-def UpdateLocalVariableDict(self, existing_dict):
+def createNotebookContext(self):
   """
-  Function to update local_varibale_dict for a Data Notebook
+  Function to create an empty notebook context.
   """
-  new_dict = self.Base_addLocalVariableDict()
-  for key, val in existing_dict['variables'].iteritems():
-    new_dict['variables'][key] = val
-  for key, val in existing_dict['imports'].iteritems():
-    new_dict['imports'][key] = val
-  for key, val in existing_dict['setup'].iteritems():
-    new_dict['setup'][key] = val
-  return new_dict
+  return {'variables': {}, 'setup': {}}
 
 class ObjectProcessor(object):
   '''

bt5/erp5_data_notebook/PropertySheetTemplateItem/portal_property_sheets/JupyterNotebook/local_varibale_property.xml → bt5/erp5_data_notebook/PropertySheetTemplateItem/portal_property_sheets/JupyterNotebook/notebook_context_property.xml

@@ -10,7 +10,7 @@
             <key> <string>categories</string> </key>
             <value>
               <tuple>
-                <string>elementary_type/string</string>
+                <string>elementary_type/object</string>
               </tuple>
             </value>
         </item>
@@ -22,7 +22,7 @@
         </item>
         <item>
             <key> <string>id</string> </key>
-            <value> <string>local_varibale_property</string> </value>
+            <value> <string>notebook_context_property</string> </value>
         </item>
         <item>
             <key> <string>portal_type</string> </key>

bt5/erp5_data_notebook/SkinTemplateItem/portal_skins/erp5_data_notebook/Base_addLocalVariableDict.xml → bt5/erp5_data_notebook/SkinTemplateItem/portal_skins/erp5_data_notebook/Base_createNotebookContext.xml

@@ -8,7 +8,7 @@
       <dictionary>
         <item>
             <key> <string>_function</string> </key>
-            <value> <string>AddNewLocalVariableDict</string> </value>
+            <value> <string>createNotebookContext</string> </value>
         </item>
         <item>
             <key> <string>_module</string> </key>
@@ -16,7 +16,7 @@
         </item>
         <item>
             <key> <string>id</string> </key>
-            <value> <string>Base_addLocalVariableDict</string> </value>
+            <value> <string>Base_createNotebookContext</string> </value>
         </item>
         <item>
             <key> <string>title</string> </key>

bt5/erp5_data_notebook/SkinTemplateItem/portal_skins/erp5_data_notebook/Base_executeJupyter.py

@@ -53,33 +53,30 @@ data_notebook_line = data_notebook.DataNotebook_addDataNotebookLine(
   batch_mode=True
 )
 
-# Get active_process associated with data_notebook object
-process_id = data_notebook.getProcess()
-active_process = portal.portal_activities[process_id]
-# Add a result object to Active Process object
-result_list = active_process.getResultList()
-
-# Get local variables saves in Active Result, local varibales are saved as
-# persistent mapping object
-old_local_variable_dict = result_list[0].summary
-if not old_local_variable_dict:
-  old_local_variable_dict = context.Base_addLocalVariableDict()
+# Gets the context associated to the data notebook being used
+#
+old_notebook_context = data_notebook.getNotebookContext()
+if not old_notebook_context:
+  old_notebook_context = portal.Base_createNotebookContext()
 
 # Pass all to code Base_runJupyter external function which would execute the code
 # and returns a dict of result
-final_result = context.Base_runJupyter(python_expression, old_local_variable_dict)
+final_result = context.Base_runJupyter(python_expression, old_notebook_context)
 code_result = final_result['result_string']
-new_local_variable_dict = final_result['local_variable_dict']
+new_local_variable_dict = final_result['notebook_context']
 ename = final_result['ename']
 evalue = final_result['evalue']
 traceback = final_result['traceback']
 status = final_result['status']
 mime_type = final_result['mime_type']
 
-# Call to function to update persistent mapping object with new local variables
-# and save the variables in the Active Result pertaining to the current Data Notebook
-new_dict = context.Base_updateLocalVariableDict(new_local_variable_dict)
-result_list[0].edit(summary=new_dict)
+# Updates the context in the notebook with the resulting context of code 
+# execution.
+#
+try:
+  data_notebook.setNotebookContext(new_local_variable_dict)
+except Exception as e:
+  return context.Base_getErrorMessageForException(e, new_local_variable_dict)
 
 result = {
   u'code_result': code_result,

bt5/erp5_data_notebook/PropertySheetTemplateItem/portal_property_sheets/JupyterNotebook/process_property.xml → bt5/erp5_data_notebook/SkinTemplateItem/portal_skins/erp5_data_notebook/Base_getErrorMessageForException.xml

@@ -2,31 +2,25 @@
 <ZopeData>
   <record id="1" aka="AAAAAAAAAAE=">
     <pickle>
-      <global name="Standard Property" module="erp5.portal_type"/>
+      <global name="ExternalMethod" module="Products.ExternalMethod.ExternalMethod"/>
     </pickle>
     <pickle>
       <dictionary>
         <item>
-            <key> <string>categories</string> </key>
-            <value>
-              <tuple>
-                <string>elementary_type/string</string>
-              </tuple>
-            </value>
+            <key> <string>_function</string> </key>
+            <value> <string>getErrorMessageForException</string> </value>
         </item>
         <item>
-            <key> <string>description</string> </key>
-            <value>
-              <none/>
-            </value>
+            <key> <string>_module</string> </key>
+            <value> <string>JupyterCompile</string> </value>
         </item>
         <item>
             <key> <string>id</string> </key>
-            <value> <string>process_property</string> </value>
+            <value> <string>Base_getErrorMessageForException</string> </value>
         </item>
         <item>
-            <key> <string>portal_type</string> </key>
-            <value> <string>Standard Property</string> </value>
+            <key> <string>title</string> </key>
+            <value> <string></string> </value>
         </item>
       </dictionary>
     </pickle>

bt5/erp5_data_notebook/SkinTemplateItem/portal_skins/erp5_data_notebook/Base_runJupyter.xml

@@ -8,7 +8,7 @@
       <dictionary>
         <item>
             <key> <string>_function</string> </key>
-            <value> <string>Base_compileJupyterCode</string> </value>
+            <value> <string>Base_runJupyterCode</string> </value>
         </item>
         <item>
             <key> <string>_module</string> </key>

bt5/erp5_data_notebook/SkinTemplateItem/portal_skins/erp5_data_notebook/DataNotebookModule_addDataNotebook.py

 """
 Python script to add a new notebook to Data Notebook module.
-This script also concerns for assigning an Active Process for each data notebook
-created.
+This script also concerns for assigning an empty notebook context for each data
+notebook created.
 """
-from Products.CMFActivity.ActiveResult import ActiveResult
-
-# Comment out person in case addition of person required to Data Notebook object
-#person = context.ERP5Site_getAuthenticatedMemberPersonValue()
-
-# Create new ActiveProcess object and getting its id
-active_process = context.portal_activities.newActiveProcess()
-active_process_id = active_process.getId()
-
-# Creating new dictionary via external method to save results in ZODB
-new_dict = context.Base_addLocalVariableDict()
-# Add new ActiveResult object and add it to the activeprocess concerned with ...
-# Data Notebook in concern
-result = ActiveResult(summary=new_dict)
-active_process.activateResult(result)
+# Creating new context via external method to save results in ZODB
+notebook_context = context.Base_createNotebookContext()
 
 # Create new notebook
 notebook = context.newContent(
     title=title,
     reference=reference,
-    process=active_process_id,
+    notebook_context=notebook_context,
     portal_type='Data Notebook'
   )
 

bt5/erp5_data_notebook/TestTemplateItem/portal_components/test.erp5.testExecuteJupyter.py

@@ -110,7 +110,7 @@ portal.%s()
     # 
     result = portal.Base_runJupyter(
       jupyter_code=jupyter_code, 
-      old_local_variable_dict=portal.Base_addLocalVariableDict()
+      old_notebook_context=portal.Base_createNotebookContext()
     )
     
     self.assertEquals(result['ename'], 'NameError')
@@ -280,10 +280,10 @@ portal.%s()
     self.assertEquals(result['ename'], 'NameError')
     self.assertEquals(result['code_result'], None)
 
-  def testBaseExecuteJupyterSaveActiveResult(self):
+  def testBaseExecuteJupyterSaveNotebookContext(self):
     """
-    Test if the result is being saved inside active_process and the user can
-    access the loacl variable and execute python expression on them
+    Test if user context is being saved in the notebook_context property and the 
+    user can access access and execute python code on it.
     """
     portal = self.portal
     self.login('dev_user')
@@ -303,12 +303,9 @@ portal.%s()
                                           reference=reference
                                           )
     notebook = notebook_list[0]
-    process_id = notebook.getProcess()
-    active_process = portal.portal_activities[process_id]
-    result_list = active_process.getResultList()
-    local_variable_dict = result_list[0].summary['variables']
+    notebook_context = notebook.getNotebookContext()['variables']
     result = {'a':2, 'b':3}
-    self.assertDictContainsSubset(result, local_variable_dict)
+    self.assertDictContainsSubset(result, notebook_context)
 
   def testBaseExecuteJupyterRerunWithPreviousLocalVariables(self):
     """
@@ -340,7 +337,7 @@ portal.%s()
 
   def testSavingModuleObjectLocalVariables(self):
     """
-    Test to check the saving of module objects in local_variable_dict
+    Test to check the saving of module objects in notebook_context
     and if they work as expected.
     """
     portal = self.portal
@@ -390,13 +387,13 @@ image = context.portal_catalog.getResultValue(portal_type='Image',reference='%s'
 context.Base_renderAsHtml(image)
 """%reference
 
-    local_variable_dict = {'setup' : {}, 'variables' : {}}
+    notebook_context = {'setup' : {}, 'variables' : {}}
     result = self.portal.Base_runJupyter(
       jupyter_code=jupyter_code,
-      old_local_variable_dict=local_variable_dict
+      old_notebook_context=notebook_context
       )
 
-    self.assertEquals(result['result_string'].rstrip(), data_template % base64.b64encode(data))
+    self.assertTrue((data_template % base64.b64encode(data)) in result['result_string'])
     # Mime_type shouldn't be  image/png just because of filename, instead it is
     # dependent on file and file data
     self.assertNotEqual(result['mime_type'], 'image/png')