[PATCH 1 of 2 cubicweb] [migrations/pdb] add to every failing migration operation a "p(db)" option

Laurent Peuch cortex at worlddomination.be
Thu Oct 24 17:48:01 CEST 2019


On Thu, Oct 24, 2019 at 10:25:42AM +0200, Philippe Pepiot wrote:
> On 22/10/2019, Laurent Peuch wrote:
> > On Tue, Oct 22, 2019 at 11:19:41AM +0200, Philippe Pepiot wrote:
> > > On 18/10/2019, Laurent Peuch wrote:
> > > > # HG changeset patch
> > > > # User Laurent Peuch <cortex at worlddomination.be>
> > > > # Date 1558564411 -7200
> > > > #      Thu May 23 00:33:31 2019 +0200
> > > > # Node ID e5f34c8b3a5929dbd7ee58411ea4fb8fecb957e0
> > > > # Parent  5c432a7fc442e170c9005666131b01a88a5c9334
> > > > # Available At https://hg.logilab.org/users/lpeuch/cubicweb
> > > > #              hg pull https://hg.logilab.org/users/lpeuch/cubicweb -r e5f34c8b3a59
> > > > # EXP-Topic improve-migrate-command
> > > > [migrations/pdb] add to every failing migration operation a "p(db)" option
> > > >
> > > > Instead, the migration command will just crash without offering the possibility
> > > > of the user to debug or continue the migration.
> > >
> > > Isn't this already catched by InstanceCommand.run() exception handling, when
> > > --pdb is set ?
> > >
> > > I'm not sure we want a pdb per "migration command", a pdb on the whole
> > > migration, and only if --pdb is set is enough I think.
> >
> > No, the usage is not the same:
> >
> > - --pdb is for post-mortem on failed commands, like everything crash
> >   and you can debug if you want
> > - "p(db)" asked during on a migration command is per migration command
> >   and won't crash the whole migration, you can actually fix things and
> >   move on to the next migration commands
> >
> > I added it to every command because some were missing it which cause
> > the whole migration to crash without any chance to debug it (and --pdb
> > wasn't there and the CLI command always returned "0" (see my others
> > patches) and it was a nightmare to debug because even "ipython --pdb"
> > didn't worked ^^')
>
> Hmm ok. I'm not sure I'll use this, but I'm ok to move forward on this
> if others think it's useful.
>
> When debugging with pdb in cubicweb, mostly in tests with pytest --pdb,
> I have an issue because a failing database operation rollback the
> transaction and close the connection. So "cnx" cannot be used, a
> workaround is to reopen a connection but that's not straightforward.
> Doesn't your implementation of spawning a pdb in a middle of a
> failing database  migration has the same issue ?

Apparently not, I've tried to make a "rql" command failed and I'm able
to play with the cnx object without any problem:

> ipdb> cnx
> <cubicweb.server.session.Connection object at 0x7fb4a77ea198>
> ipdb> cnx.build_url()
> 'Noneview'
> ipdb> cnx.execute("fail")
> *** rql._exceptions.RQLSyntaxError: fail;
> at: ('<f.286>', 1, 0)
> Trying to find one of DELETE, INSERT, SET, r"\(", DISTINCT, E_TYPE


> > > Regarding https://www.cubicweb.org/17219772 , and in my opinion existing
> > > questions are already too much questions. What do you (and others)
> > > think about dropping interactive upgrade instead ?
> > >
> > > Also, as a user my concern is much more about transactional sql upgrades, I
> > > expect a single migration either succeed or fail without leaving me in
> > > an intermediate state. This goal doesn't seem consistent with upgrade
> > > command going more interactive.
> > >
> > > Most of (all ?) our production instances run upgrade in a non-interactive mode.
> > > Sometimes we even don't have access to the server.
> > >
> > > I think we should discuss this, sorry to not have seen this ticket before.
> >
> > I don't have any strong opinion on this one but I think you have 2
> > differents situations:
> >
> > - you are working on writing a migration or migration a (possibly old)
> >   instance and using the interactive mode your work will be easier
> > - the case you've described where, indeed, you want a win or fail
> >
> > In my opinion, everything that helps making development easier is good
> > to take but we might want to change the UX here to reflect this
> > better? Like swapping the "interactive by default" to a
> > "--interactive".
>
> Good idea. Or maybe test if we have a tty before asking something to the
> user.

We aren't already doing that? That would be surprising but that would
be a good thing to do yes.

-- 

Laurent Peuch -- Bram



More information about the cubicweb-devel mailing list