[PATCH 2 of 2 cubicweb] [migrations] only call once the exception handling mecanism on failed command

Laurent Peuch cortex at worlddomination.be
Fri Oct 18 05:11:45 CEST 2019

# HG changeset patch
# User Laurent Peuch <cortex at worlddomination.be>
# Date 1571366385 -7200
#      Fri Oct 18 04:39:45 2019 +0200
# Node ID b967fd2b9878866f041328b23993d3797b3fe5b7
# Parent  e5f34c8b3a5929dbd7ee58411ea4fb8fecb957e0
# Available At https://hg.logilab.org/users/lpeuch/cubicweb
#              hg pull https://hg.logilab.org/users/lpeuch/cubicweb -r b967fd2b9878
# EXP-Topic improve-migrate-command
[migrations] only call once the exception handling mecanism on failed command

Following the " add to every failing migration operation a "p(db)" option"
decorator, ensure that the self.confirm with the "p(db)" option is only called
once when one command will called another or more other commands that also have
the @ask_abort_with_pdb decorator otherwise the same question could be asked
several time for the same command.

On the implementation note I've decided to use an explicite declaration method
instead of using advanced methods like parsing the call stack to detect the
decorator. This code is much simplier and will avoid maintaince burden.

diff --git a/cubicweb/server/migractions.py b/cubicweb/server/migractions.py
--- a/cubicweb/server/migractions.py
+++ b/cubicweb/server/migractions.py
@@ -80,12 +80,26 @@ class ClearGroupMap(hook.Hook):
 def ask_abort_with_pdb(function):
     def _wrap(self, *args, **kwargs):
+        # "top_function" kwargs allows to only call the exception handle
+        # mecanism in the most top function called to avoid having the same
+        # question asked several time on "Y" answer
+        if "top_function" in kwargs:
+            top_function = kwargs["top_function"]
+            del kwargs["top_function"]
+        else:
+            top_function = True
             to_return = function(self, *args, **kwargs)
         except Exception:
-            _, ex, traceback_ = sys.exc_info()
-            traceback.print_exc()
-            if self.confirm('abort?', pdb=True, traceback=traceback_):
+            # only handle that at the top function call to be
+            # closer to the user excpected behavior
+            if top_function:
+                _, ex, traceback_ = sys.exc_info()
+                traceback.print_exc()
+                if self.confirm('abort?', pdb=True, traceback=traceback_):
+                    raise
+            else:
             return to_return
@@ -642,7 +656,7 @@ class ServerMigrationHelper(MigrationHel
     def cmd_add_cube(self, cube, update_database=True):
-        self.cmd_add_cubes((cube,), update_database)
+        self.cmd_add_cubes((cube,), update_database, top_function=False)
     def cmd_add_cubes(self, cubes, update_database=True):
@@ -656,7 +670,8 @@ class ServerMigrationHelper(MigrationHel
         for cube in newcubes:
             self.cmd_set_property('system.version.' + cube,
-                                  self.config.cube_version(cube))
+                                  self.config.cube_version(cube),
+                                  top_function=False)
             # ensure added cube is in config cubes
             # XXX worth restoring on error?
             if cube not in self.config._cubes:
@@ -677,12 +692,12 @@ class ServerMigrationHelper(MigrationHel
         # add new entity and relation types
         for rschema in newcubes_schema.relations():
             if rschema not in self.repo.schema:
-                self.cmd_add_relation_type(rschema.type, commit=False)
+                self.cmd_add_relation_type(rschema.type, commit=False, top_function=False)
         toadd = [eschema for eschema in newcubes_schema.entities()
                  if eschema not in self.repo.schema]
         for eschema in order_eschemas(toadd):
-            self.cmd_add_entity_type(eschema.type)
+            self.cmd_add_entity_type(eschema.type, top_function=False)
         # check if attributes has been added to existing entities
         for rschema in newcubes_schema.relations():
@@ -699,7 +714,7 @@ class ServerMigrationHelper(MigrationHel
                 if not (fromtype in new or totype in new or rschema in new):
                 self.cmd_add_relation_definition(str(fromtype), rschema.type,
-                                                 str(totype))
+                                                 str(totype), top_function=False)
         # execute post-create files
         for cube in reversed(newcubes):
             with self.cnx.allow_all_hooks_but():
@@ -721,11 +736,11 @@ class ServerMigrationHelper(MigrationHel
         # remove cubes'entity and relation types
         for rschema in fsschema.relations():
             if rschema not in removedcubes_schema and rschema in reposchema:
-                self.cmd_drop_relation_type(rschema.type)
+                self.cmd_drop_relation_type(rschema.type, top_function=False)
         toremove = [eschema for eschema in fsschema.entities()
                     if eschema not in removedcubes_schema and eschema in reposchema]
         for eschema in reversed(order_eschemas(toremove)):
-            self.cmd_drop_entity_type(eschema.type)
+            self.cmd_drop_entity_type(eschema.type, top_function=False)
         for rschema in fsschema.relations():
             if rschema in removedcubes_schema and rschema in reposchema:
                 # check if attributes/relations has been added to entities from
@@ -734,7 +749,7 @@ class ServerMigrationHelper(MigrationHel
                     if (fromtype, totype) not in removedcubes_schema[rschema.type].rdefs and \
                        (fromtype, totype) in reposchema[rschema.type].rdefs:
-                            str(fromtype), rschema.type, str(totype))
+                            str(fromtype), rschema.type, str(totype), top_function=False)
         # execute post-remove files
         for cube in reversed(removedcubes):
             self.cmd_exec_event_script('postremove', cube)
@@ -750,7 +765,8 @@ class ServerMigrationHelper(MigrationHel
         if attrtype is None:
             rschema = self.fs_schema.rschema(attrname)
             attrtype = rschema.objects(etype)[0]
-        self.cmd_add_relation_definition(etype, attrname, attrtype, commit=commit)
+        self.cmd_add_relation_definition(etype, attrname, attrtype, commit=commit,
+                                         top_function=False)
     def cmd_drop_attribute(self, etype, attrname, commit=True):
@@ -766,7 +782,7 @@ class ServerMigrationHelper(MigrationHel
                 etype, attrname))
             self.cmd_drop_relation_definition(etype, attrname, attrtype,
-                                              commit=commit)
+                                              commit=commit, top_function=False)
     def cmd_rename_attribute(self, etype, oldname, newname, commit=True):
@@ -779,7 +795,7 @@ class ServerMigrationHelper(MigrationHel
         attrtype = eschema.destination(newname)
         # have to commit this first step anyway to get the definition
         # actually in the schema
-        self.cmd_add_attribute(etype, newname, attrtype, commit=True)
+        self.cmd_add_attribute(etype, newname, attrtype, commit=True, top_function=False)
         # skipp NULL values if the attribute is required
         rql = 'SET X %s VAL WHERE X is %s, X %s VAL' % (newname, etype, oldname)
         card = eschema.rdef(newname).cardinality[0]
@@ -790,7 +806,7 @@ class ServerMigrationHelper(MigrationHel
         # XXX if old attribute was fti indexed but not the new one old value
         # won't be removed from the index (this occurs on other kind of
         # fulltextindexed change...)
-        self.cmd_drop_attribute(etype, oldname, commit=commit)
+        self.cmd_drop_attribute(etype, oldname, commit=commit, top_function=False)
     def cmd_add_entity_type(self, etype, auto=True, commit=True):
@@ -830,10 +846,10 @@ class ServerMigrationHelper(MigrationHel
             if rschema.type in META_RTYPES:
             if attrschema.type not in instschema:
-                self.cmd_add_entity_type(attrschema.type, False, False)
+                self.cmd_add_entity_type(attrschema.type, False, False, top_function=False)
             if rschema.type not in instschema:
                 # need to add the relation type
-                self.cmd_add_relation_type(rschema.type, False, commit=False)
+                self.cmd_add_relation_type(rschema.type, False, commit=False, top_function=False)
             # register relation definition
             rdef = self._get_rdef(rschema, eschema, eschema.destination(rschema))
             ss.execschemarql(execute, rdef, ss.rdef2rql(rdef, cstrtypemap, groupmap),)
@@ -870,7 +886,8 @@ class ServerMigrationHelper(MigrationHel
                                 (subjschema, objschema) in instschema[rschema].rdefs)):
-                            subjschema.type, rschema.type, objschema.type)
+                            subjschema.type, rschema.type, objschema.type,
+                            top_function=False)
         if auto:
             # we have commit here to get relation types actually in the schema
@@ -891,7 +908,8 @@ class ServerMigrationHelper(MigrationHel
                         # need to add the relation type and to commit to get it
                         # actually in the schema
-                        self.cmd_add_relation_type(rschema.type, False, commit=False)
+                        self.cmd_add_relation_type(rschema.type, False, commit=False,
+                                                   top_function=False)
                         rtypeadded = True
                     # register relation definition
                     # remember this two avoid adding twice non symmetric relation
@@ -915,7 +933,8 @@ class ServerMigrationHelper(MigrationHel
                     if not rtypeadded:
                         # need to add the relation type and to commit to get it
                         # actually in the schema
-                        self.cmd_add_relation_type(rschema.type, False, commit=False)
+                        self.cmd_add_relation_type(rschema.type, False, commit=False,
+                                                   top_function=False)
                         rtypeadded = True
                     elif (targettype, rschema.type, etype) in added:
@@ -1071,7 +1090,7 @@ class ServerMigrationHelper(MigrationHel
                 if (subj, obj) in done:
                 done.add((subj, obj))
-                self.cmd_add_relation_definition(subj, rtype, obj)
+                self.cmd_add_relation_definition(subj, rtype, obj, top_function=False)
             if rtype in META_RTYPES:
                 # if the relation is in META_RTYPES, ensure we're adding it for
                 # all entity types *in the persistent schema*, not only those in
@@ -1119,11 +1138,11 @@ class ServerMigrationHelper(MigrationHel
                                 ' do you really want to drop it?' % oldname,
-        self.cmd_add_relation_type(newname, commit=False)
+        self.cmd_add_relation_type(newname, commit=False, top_function=False)
         if not self.repo.schema[oldname].rule:
             self.rqlexec('SET X %s Y WHERE X %s Y' % (newname, oldname),
                          ask_confirm=self.verbosity >= 2)
-        self.cmd_drop_relation_type(oldname, commit=commit)
+        self.cmd_drop_relation_type(oldname, commit=commit, top_function=False)
     def cmd_add_relation_definition(self, subjtype, rtype, objtype, commit=True):
@@ -1135,7 +1154,7 @@ class ServerMigrationHelper(MigrationHel
             raise ExecutionError('Cannot add a relation definition for a '
                                  'computed relation (%s)' % rschema)
         if rtype not in self.repo.schema:
-            self.cmd_add_relation_type(rtype, addrdef=False, commit=False)
+            self.cmd_add_relation_type(rtype, addrdef=False, commit=False, top_function=False)
         if (subjtype, objtype) in self.repo.schema.rschema(rtype).rdefs:
             print('warning: relation %s %s %s is already known, skip addition' % (
                 subjtype, rtype, objtype))
@@ -1296,9 +1315,9 @@ class ServerMigrationHelper(MigrationHel
     def cmd_make_workflowable(self, etype):
         """add workflow relations to an entity type to make it workflowable"""
-        self.cmd_add_relation_definition(etype, 'in_state', 'State')
-        self.cmd_add_relation_definition(etype, 'custom_workflow', 'Workflow')
-        self.cmd_add_relation_definition('TrInfo', 'wf_info_for', etype)
+        self.cmd_add_relation_definition(etype, 'in_state', 'State', top_function=False)
+        self.cmd_add_relation_definition(etype, 'custom_workflow', 'Workflow', top_function=False)
+        self.cmd_add_relation_definition('TrInfo', 'wf_info_for', etype, top_function=False)
     def cmd_add_workflow(self, name, wfof, default=True, commit=False,
@@ -1318,7 +1337,7 @@ class ServerMigrationHelper(MigrationHel
          :rtype: `Workflow`
-        wf = self.cmd_create_entity('Workflow', name=name,
+        wf = self.cmd_create_entity('Workflow', name=name, top_function=False,
         if not isinstance(wfof, (list, tuple)):
             wfof = (wfof,)
@@ -1378,7 +1397,7 @@ class ServerMigrationHelper(MigrationHel
                 'CWProperty X WHERE X pkey %(k)s, NOT X for_user U',
                 {'k': str(pkey)}, ask_confirm=False).get_entity(0, 0)
         except Exception:
-            self.cmd_create_entity('CWProperty', pkey=str(pkey), value=value)
+            self.cmd_create_entity('CWProperty', pkey=str(pkey), value=value, top_function=False)
@@ -1487,7 +1506,7 @@ class ServerMigrationHelper(MigrationHel
         if droprequired:
             rdef.cardinality = original_cardinality
         # update repository schema
-        self.cmd_sync_schema_props_perms(rdef, syncperms=False)
+        self.cmd_sync_schema_props_perms(rdef, syncperms=False, top_function=False)
     def sqlexec(self, sql, args=None, ask_confirm=True):
         """execute the given sql if confirmed

More information about the cubicweb-devel mailing list