[v5,1/1] management: introduce replacerelations command

The replacerelations script is used to ingest relations into Patchwork's
patch database. A patch groups file is taken as input, which on each
line contains a space separated list of patchwork ids denoting a
relation. All the existing relations in Patchwork's database are removed
and the relations read from the patch groups file are ingested.

Signed-off-by: Rohit Sarkar <rohitsarkar5398@gmail.com>
---
 docs/deployment/management.rst                | 37 ++++++++++
 .../management/commands/replacerelations.py   | 72 +++++++++++++++++++
 patchwork/tests/__init__.py                   |  1 +
 patchwork/tests/test_management.py            | 54 +++++++++++++-
 4 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 patchwork/management/commands/replacerelations.py

Hi Rohit,

I have applied this: commit fe0c0ca7279e

I made the following changes:

 - The test relations directory doesn't get saved to git as git doesn't
   save empty directories. So attempting to run the tests on a clean git
   tree fails. I've just changed the code to drop TEST_RELATIONS_DIR and
   just put the temporary files in the temporary directory.

 - I was still seeing some pep8 failures. They were pretty trivial so I
   just fixed them up but in future please run
   'docker-compose --tox -e pep8' on your code before submitting it.

 - It took me a while to figure out what the map(str, ...) code was
   doing, so I added a comment explaining that this allowed us to handle
   lines with 2 or 3 patches without needing a special case.

Congratulations on getting code into Patchwork! I hope you will continue
to contribute in future.

Kind regards,
Daniel

> The replacerelations script is used to ingest relations into Patchwork's
> patch database. A patch groups file is taken as input, which on each
> line contains a space separated list of patchwork ids denoting a
> relation. All the existing relations in Patchwork's database are removed
> and the relations read from the patch groups file are ingested.
>
> Signed-off-by: Rohit Sarkar <rohitsarkar5398@gmail.com>
> ---
>  docs/deployment/management.rst                | 37 ++++++++++
>  .../management/commands/replacerelations.py   | 72 +++++++++++++++++++
>  patchwork/tests/__init__.py                   |  1 +
>  patchwork/tests/test_management.py            | 54 +++++++++++++-
>  4 files changed, 163 insertions(+), 1 deletion(-)
>  create mode 100644 patchwork/management/commands/replacerelations.py
>
> diff --git a/docs/deployment/management.rst b/docs/deployment/management.rst
> index 9c57f19..dcee9ff 100644
> --- a/docs/deployment/management.rst
> +++ b/docs/deployment/management.rst
> @@ -116,6 +116,43 @@ the :ref:`deployment installation guide <deployment-parsemail>`.
>  
>     input mbox filename. If not supplied, a patch will be read from ``stdin``.
>  
> +replacerelations
> +~~~~~~~~~~~~~~~~
> +
> +.. program:: manage.py replacerelations
> +
> +Parse a patch groups file and store any relation found
> +
> +.. code-block:: shell
> +
> +    ./manage.py replacerelations <infile>
> +
> +This is a script used to ingest relations into Patchwork.
> +
> +A patch groups file contains on each line a list of space separated patch IDs
> +of patches that form a relation.
> +
> +For example, consider the contents of a sample patch groups file::
> +
> +    1 3 5
> +    2
> +    7 10 11 12
> +
> +In this case the script will identify 2 relations, (1, 3, 5) and
> +(7, 10, 11, 12). The single patch ID "2" on the second line is ignored as a
> +relation always consists of more than 1 patch.
> +
> +Further, if a patch ID in the patch groups file does not exist in the database
> +of the Patchwork instance, that patch ID will be silently ignored while forming
> +the relations.
> +
> +Running this script will remove all existing relations and replace them with
> +the relations found in the file.
> +
> +.. option:: infile
> +
> +    input patch groups file.
> +
>  rehash
>  ~~~~~~
>  
> diff --git a/patchwork/management/commands/replacerelations.py b/patchwork/management/commands/replacerelations.py
> new file mode 100644
> index 0000000..0cc4d0a
> --- /dev/null
> +++ b/patchwork/management/commands/replacerelations.py
> @@ -0,0 +1,72 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2020 Rohit Sarkar <rohitsarkar5398@gmail.com>
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import logging
> +import os
> +import sys
> +
> +from django.db import transaction
> +from django.core.management.base import BaseCommand
> +
> +from patchwork.models import Patch
> +from patchwork.models import PatchRelation
> +
> +logger = logging.getLogger(__name__)
> +
> +class Command(BaseCommand):
> +    help = 'Parse a relations file generated by PaStA and replace existing relations with the ones parsed'
> +
> +    def add_arguments(self, parser):
> +        parser.add_argument(
> +            'infile',
> +            help='input relations filename')
> +
> +    def handle(self, *args, **options):
> +        verbosity = int(options['verbosity'])
> +        if not verbosity:
> +            level = logging.CRITICAL
> +        elif verbosity == 1:
> +            level = logging.ERROR
> +        elif verbosity == 2:
> +            level = logging.INFO
> +        else:
> +            level = logging.DEBUG
> +
> +        logger.setLevel(level)
> +
> +        path = args and args[0] or options['infile']
> +        if not os.path.exists(path):
> +            logger.error('Invalid path: %s', path)
> +            sys.exit(1)
> +
> +
> +        with open(path, 'r') as f:
> +            lines = f.readlines()
> +
> +        # filter out trailing empty lines
> +        while len(lines) and not lines[-1]:
> +            lines.pop()
> +
> +        relations = [line.split(' ') for line in lines]
> +
> +        with transaction.atomic():
> +            PatchRelation.objects.all().delete()
> +            count = len(relations)
> +            ingested = 0
> +            logger.info('Parsing %d relations' % count)
> +            for i, patch_ids in enumerate(relations):
> +                related_patches = Patch.objects.filter(id__in=patch_ids)
> +
> +                if len(related_patches) > 1:
> +                    relation = PatchRelation()
> +                    relation.save()
> +                    related_patches.update(related=relation)
> +                    ingested += 1
> +
> +                if i % 10 == 0:
> +                    self.stdout.write('%06d/%06d\r' % (i, count), ending='')
> +                    self.stdout.flush()
> +
> +            self.stdout.write('Ingested %d relations' % ingested)
> diff --git a/patchwork/tests/__init__.py b/patchwork/tests/__init__.py
> index 8f78ea7..83358a6 100644
> --- a/patchwork/tests/__init__.py
> +++ b/patchwork/tests/__init__.py
> @@ -8,3 +8,4 @@ import os
>  TEST_MAIL_DIR = os.path.join(os.path.dirname(__file__), 'mail')
>  TEST_PATCH_DIR = os.path.join(os.path.dirname(__file__), 'patches')
>  TEST_FUZZ_DIR = os.path.join(os.path.dirname(__file__), 'fuzztests')
> +TEST_RELATIONS_DIR = os.path.join(os.path.dirname(__file__), 'relations')
> diff --git a/patchwork/tests/test_management.py b/patchwork/tests/test_management.py
> index 66c6bad..d33a9f3 100644
> --- a/patchwork/tests/test_management.py
> +++ b/patchwork/tests/test_management.py
> @@ -5,13 +5,14 @@
>  
>  import os
>  import sys
> +import tempfile
>  from io import StringIO
>  
>  from django.core.management import call_command
>  from django.test import TestCase
>  
>  from patchwork import models
> -from patchwork.tests import TEST_MAIL_DIR
> +from patchwork.tests import TEST_MAIL_DIR, TEST_RELATIONS_DIR
>  from patchwork.tests import utils
>  
>  
> @@ -124,3 +125,54 @@ class ParsearchiveTest(TestCase):
>  
>          self.assertIn('Processed 1 messages -->', out.getvalue())
>          self.assertIn('  1 dropped', out.getvalue())
> +
> +class ReplacerelationsTest(TestCase):
> +    def test_invalid_path(self):
> +        out = StringIO()
> +        with self.assertRaises(SystemExit) as exc:
> +            call_command('replacerelations', 'xyz123random', '-v 0', stdout=out)
> +        self.assertEqual(exc.exception.code, 1)
> +
> +    def test_valid_relations(self):
> +        test_submitter = utils.create_person()
> +        utils.create_patches(8, submitter=test_submitter)
> +        patch_ids = (
> +                    models.Patch.objects
> +                    .filter(submitter=test_submitter)
> +                    .values_list('id', flat=True)
> +                )
> +
> +        with tempfile.NamedTemporaryFile(delete=False,
> +                                         dir=TEST_RELATIONS_DIR,
> +                                         mode='w+') as f1:
> +            for i in range(0, len(patch_ids), 3):
> +                f1.write('%s\n' % ' '.join(map(str, patch_ids[i:i+3])))
> +
> +
> +        out = StringIO()
> +        call_command('replacerelations',
> +                     os.path.join(TEST_RELATIONS_DIR,
> +                                  f1.name),
> +                                  stdout=out)
> +        self.assertEqual(models.PatchRelation.objects.count(), 3)
> +        os.unlink(f1.name)
> +
> +        patch_ids_with_missing = (
> +                    list(patch_ids) +
> +                    [i for i in range(max(patch_ids), max(patch_ids) + 3)]
> +                )
> +        with tempfile.NamedTemporaryFile(delete=False,
> +                                         dir=TEST_RELATIONS_DIR,
> +                                         mode='w+') as f2:
> +            for i in range(0, len(patch_ids_with_missing), 3):
> +                f2.write(
> +                        '%s\n' % ' '.join(map(str,
> +                                              patch_ids_with_missing[i:i+3]))
> +                        )
> +
> +        call_command('replacerelations',
> +                     os.path.join(TEST_RELATIONS_DIR,
> +                                  f2.name),
> +                                  stdout=out)
> +        self.assertEqual(models.PatchRelation.objects.count(), 3)
> +        os.unlink(f2.name)
> -- 
> 2.27.0

Hey Daniel,
On Mon, Aug 10, 2020 at 11:52:28AM +1000, Daniel Axtens wrote:
> Hi Rohit,
> 
> I have applied this: commit fe0c0ca7279e
> 
> I made the following changes:
> 
>  - The test relations directory doesn't get saved to git as git doesn't
>    save empty directories. So attempting to run the tests on a clean git
>    tree fails. I've just changed the code to drop TEST_RELATIONS_DIR and
>    just put the temporary files in the temporary directory.
Ah, nice catch.

>  - I was still seeing some pep8 failures. They were pretty trivial so I
>    just fixed them up but in future please run
>    'docker-compose --tox -e pep8' on your code before submitting it.
Sorry for that. Sure, thanks!

>  - It took me a while to figure out what the map(str, ...) code was
>    doing, so I added a comment explaining that this allowed us to handle
>    lines with 2 or 3 patches without needing a special case.
Makes sense, thanks.

> Congratulations on getting code into Patchwork! I hope you will continue
> to contribute in future.
I will definitely try to be an active contributor to Patchwork.

> Kind regards,
> Daniel
Thanks again :) ,
Rohit
> > The replacerelations script is used to ingest relations into Patchwork's
> > patch database. A patch groups file is taken as input, which on each
> > line contains a space separated list of patchwork ids denoting a
> > relation. All the existing relations in Patchwork's database are removed
> > and the relations read from the patch groups file are ingested.
> >
> > Signed-off-by: Rohit Sarkar <rohitsarkar5398@gmail.com>
> > ---
> >  docs/deployment/management.rst                | 37 ++++++++++
> >  .../management/commands/replacerelations.py   | 72 +++++++++++++++++++
> >  patchwork/tests/__init__.py                   |  1 +
> >  patchwork/tests/test_management.py            | 54 +++++++++++++-
> >  4 files changed, 163 insertions(+), 1 deletion(-)
> >  create mode 100644 patchwork/management/commands/replacerelations.py
> >
> > diff --git a/docs/deployment/management.rst b/docs/deployment/management.rst
> > index 9c57f19..dcee9ff 100644
> > --- a/docs/deployment/management.rst
> > +++ b/docs/deployment/management.rst
> > @@ -116,6 +116,43 @@ the :ref:`deployment installation guide <deployment-parsemail>`.
> >  
> >     input mbox filename. If not supplied, a patch will be read from ``stdin``.
> >  
> > +replacerelations
> > +~~~~~~~~~~~~~~~~
> > +
> > +.. program:: manage.py replacerelations
> > +
> > +Parse a patch groups file and store any relation found
> > +
> > +.. code-block:: shell
> > +
> > +    ./manage.py replacerelations <infile>
> > +
> > +This is a script used to ingest relations into Patchwork.
> > +
> > +A patch groups file contains on each line a list of space separated patch IDs
> > +of patches that form a relation.
> > +
> > +For example, consider the contents of a sample patch groups file::
> > +
> > +    1 3 5
> > +    2
> > +    7 10 11 12
> > +
> > +In this case the script will identify 2 relations, (1, 3, 5) and
> > +(7, 10, 11, 12). The single patch ID "2" on the second line is ignored as a
> > +relation always consists of more than 1 patch.
> > +
> > +Further, if a patch ID in the patch groups file does not exist in the database
> > +of the Patchwork instance, that patch ID will be silently ignored while forming
> > +the relations.
> > +
> > +Running this script will remove all existing relations and replace them with
> > +the relations found in the file.
> > +
> > +.. option:: infile
> > +
> > +    input patch groups file.
> > +
> >  rehash
> >  ~~~~~~
> >  
> > diff --git a/patchwork/management/commands/replacerelations.py b/patchwork/management/commands/replacerelations.py
> > new file mode 100644
> > index 0000000..0cc4d0a
> > --- /dev/null
> > +++ b/patchwork/management/commands/replacerelations.py
> > @@ -0,0 +1,72 @@
> > +# Patchwork - automated patch tracking system
> > +# Copyright (C) 2020 Rohit Sarkar <rohitsarkar5398@gmail.com>
> > +#
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +import logging
> > +import os
> > +import sys
> > +
> > +from django.db import transaction
> > +from django.core.management.base import BaseCommand
> > +
> > +from patchwork.models import Patch
> > +from patchwork.models import PatchRelation
> > +
> > +logger = logging.getLogger(__name__)
> > +
> > +class Command(BaseCommand):
> > +    help = 'Parse a relations file generated by PaStA and replace existing relations with the ones parsed'
> > +
> > +    def add_arguments(self, parser):
> > +        parser.add_argument(
> > +            'infile',
> > +            help='input relations filename')
> > +
> > +    def handle(self, *args, **options):
> > +        verbosity = int(options['verbosity'])
> > +        if not verbosity:
> > +            level = logging.CRITICAL
> > +        elif verbosity == 1:
> > +            level = logging.ERROR
> > +        elif verbosity == 2:
> > +            level = logging.INFO
> > +        else:
> > +            level = logging.DEBUG
> > +
> > +        logger.setLevel(level)
> > +
> > +        path = args and args[0] or options['infile']
> > +        if not os.path.exists(path):
> > +            logger.error('Invalid path: %s', path)
> > +            sys.exit(1)
> > +
> > +
> > +        with open(path, 'r') as f:
> > +            lines = f.readlines()
> > +
> > +        # filter out trailing empty lines
> > +        while len(lines) and not lines[-1]:
> > +            lines.pop()
> > +
> > +        relations = [line.split(' ') for line in lines]
> > +
> > +        with transaction.atomic():
> > +            PatchRelation.objects.all().delete()
> > +            count = len(relations)
> > +            ingested = 0
> > +            logger.info('Parsing %d relations' % count)
> > +            for i, patch_ids in enumerate(relations):
> > +                related_patches = Patch.objects.filter(id__in=patch_ids)
> > +
> > +                if len(related_patches) > 1:
> > +                    relation = PatchRelation()
> > +                    relation.save()
> > +                    related_patches.update(related=relation)
> > +                    ingested += 1
> > +
> > +                if i % 10 == 0:
> > +                    self.stdout.write('%06d/%06d\r' % (i, count), ending='')
> > +                    self.stdout.flush()
> > +
> > +            self.stdout.write('Ingested %d relations' % ingested)
> > diff --git a/patchwork/tests/__init__.py b/patchwork/tests/__init__.py
> > index 8f78ea7..83358a6 100644
> > --- a/patchwork/tests/__init__.py
> > +++ b/patchwork/tests/__init__.py
> > @@ -8,3 +8,4 @@ import os
> >  TEST_MAIL_DIR = os.path.join(os.path.dirname(__file__), 'mail')
> >  TEST_PATCH_DIR = os.path.join(os.path.dirname(__file__), 'patches')
> >  TEST_FUZZ_DIR = os.path.join(os.path.dirname(__file__), 'fuzztests')
> > +TEST_RELATIONS_DIR = os.path.join(os.path.dirname(__file__), 'relations')
> > diff --git a/patchwork/tests/test_management.py b/patchwork/tests/test_management.py
> > index 66c6bad..d33a9f3 100644
> > --- a/patchwork/tests/test_management.py
> > +++ b/patchwork/tests/test_management.py
> > @@ -5,13 +5,14 @@
> >  
> >  import os
> >  import sys
> > +import tempfile
> >  from io import StringIO
> >  
> >  from django.core.management import call_command
> >  from django.test import TestCase
> >  
> >  from patchwork import models
> > -from patchwork.tests import TEST_MAIL_DIR
> > +from patchwork.tests import TEST_MAIL_DIR, TEST_RELATIONS_DIR
> >  from patchwork.tests import utils
> >  
> >  
> > @@ -124,3 +125,54 @@ class ParsearchiveTest(TestCase):
> >  
> >          self.assertIn('Processed 1 messages -->', out.getvalue())
> >          self.assertIn('  1 dropped', out.getvalue())
> > +
> > +class ReplacerelationsTest(TestCase):
> > +    def test_invalid_path(self):
> > +        out = StringIO()
> > +        with self.assertRaises(SystemExit) as exc:
> > +            call_command('replacerelations', 'xyz123random', '-v 0', stdout=out)
> > +        self.assertEqual(exc.exception.code, 1)
> > +
> > +    def test_valid_relations(self):
> > +        test_submitter = utils.create_person()
> > +        utils.create_patches(8, submitter=test_submitter)
> > +        patch_ids = (
> > +                    models.Patch.objects
> > +                    .filter(submitter=test_submitter)
> > +                    .values_list('id', flat=True)
> > +                )
> > +
> > +        with tempfile.NamedTemporaryFile(delete=False,
> > +                                         dir=TEST_RELATIONS_DIR,
> > +                                         mode='w+') as f1:
> > +            for i in range(0, len(patch_ids), 3):
> > +                f1.write('%s\n' % ' '.join(map(str, patch_ids[i:i+3])))
> > +
> > +
> > +        out = StringIO()
> > +        call_command('replacerelations',
> > +                     os.path.join(TEST_RELATIONS_DIR,
> > +                                  f1.name),
> > +                                  stdout=out)
> > +        self.assertEqual(models.PatchRelation.objects.count(), 3)
> > +        os.unlink(f1.name)
> > +
> > +        patch_ids_with_missing = (
> > +                    list(patch_ids) +
> > +                    [i for i in range(max(patch_ids), max(patch_ids) + 3)]
> > +                )
> > +        with tempfile.NamedTemporaryFile(delete=False,
> > +                                         dir=TEST_RELATIONS_DIR,
> > +                                         mode='w+') as f2:
> > +            for i in range(0, len(patch_ids_with_missing), 3):
> > +                f2.write(
> > +                        '%s\n' % ' '.join(map(str,
> > +                                              patch_ids_with_missing[i:i+3]))
> > +                        )
> > +
> > +        call_command('replacerelations',
> > +                     os.path.join(TEST_RELATIONS_DIR,
> > +                                  f2.name),
> > +                                  stdout=out)
> > +        self.assertEqual(models.PatchRelation.objects.count(), 3)
> > +        os.unlink(f2.name)
> > -- 
> > 2.27.0