[RFC PATCH 0/1] add parserelations management command

[Rohit Sarkar]

Hey Daniel,
On Fri, Jun 12, 2020 at 12:39:09AM +1000, Daniel Axtens wrote:
> Rohit Sarkar <rohitsarkar5398 at gmail.com> writes:
> 
> > The parserelations command will be used to bulk import patch relations into
> > Patchwork using a "patch groups" file.
> > The patch groups file is structured such that each line contains a
> > relation and an optional upstream commit reference.
> > Eg patch groups file contents.
> > 1 3 4
> > 2
> > 5 9 10 => <commit-hash-1> <commit-hash-2>
> >
> > In this case 2 relations will be ingested into Patchwork, (1,3,4) and
> > (5,9,10). Further group (5,9,10) also points to two upstream commit
> > references.
> 
> > Note that before ingesting the relations the existing relations in the
> > DB are removed.
> 
> I'm not super keen on this bit, it seems too easy to shoot yourself in
> the foot.
> 
> Can we restrict it to purging the relations of projects that contain the
> patches? And require the user to specify the project they expect to be
> purged on the command line?

I agree. But relations can be across projects. We did think a bit about
this and decided to go with this approach as a v1. In our next steps we
were considering some sort of differential approach towards pushing the
relations into Patchwork, ie, how do we go from state A of relations in
Patchwork to a target state B. This is non trivial as a patch can belong
to only one relation. We need some more discussion about the design of
the "differential" approach.

> > Currently the commit references are not used, as in Patchwork's model a
> > patch points to a single commit reference.
> 
> If this patch groups file format is something informal that PaStA
> generates, and the commit refs are going to be ignored, would it be
> simpler to just process them out beforehand with shell tools in your
> PaStA integration?
> 
> e.g. whichever of these is most comprehensible to you:
> 
> echo '5 9 10 => f00df00d cafecafe' | cut -d= -f1
> echo '5 9 10 => f00df00d cafecafe' | awk -F"=>" '{print $1}'
> echo '5 9 10 => f00df00d cafecafe' | sed -E 's/([^=]+) =>.*/\1/'
> 
> Alternatively, if the patch groups file is a bit more clearly defined,
> I'm happy to have the code to ignore the commit refs in patchwork:
> include a link to the specification and a more detailed explanation of
> why we're ignoring the commit refs.

We decided to remove the commit references for now as you suggested as
Patchwork isnt using them. Will update the patch to remove the code that
handles commit references in v2.

> > Some more tests will be added if this patch looks good.
> 
> I toyed a bit with the idea of suggesting that this just be kept
> out-of-tree - it's not clear who else would use it, and it's not like
> the managment interface changes very quickly. But then I realised that
> there's basically no cost for Patchwork to keep it in tree, so sure, go
> for it.
> 
> I'll give the patch a quick once-over now.
Thanks for the review!
> Regards,
> Daniel
> 
> >
> > Rohit Sarkar (1):
> >   management: introduce parserelations command
> >
> >  docs/deployment/management.rst                | 27 +++++++
> >  .../management/commands/parserelations.py     | 78 +++++++++++++++++++
> >  patchwork/tests/test_management.py            |  7 ++
> >  3 files changed, 112 insertions(+)
> >  create mode 100644 patchwork/management/commands/parserelations.py
> >
> > -- 
> > 2.23.0.385.gbc12974a89
Thanks,
Rohit