Hi Rohit,
Apologies for the delay.
My one comment is that the code silently ignores patch IDs that are not
in the database. I don't really mind that much (although I probably
would have thrown a warning?), and I'm pleased that the tests cover it,
but it's completely undocumented. Please could you add a note about that
to the documentation.
> +This is a script used to ingest relations into Patchwork.
> +A patch groups file contains on each line patchwork ids of patches that form
> a relation.
Within the context of patchwork we call them 'patch IDs', not 'patchwork ids'.
> +Eg contents of a patch groups file:
'Eg' should either have dots ('E.g.') or preferably be spelled out ("For
example, ...")
> +
> + 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).
It would also be good to highlight that a line with only 1 patch will be
ignored as it's not possible to create a relation with only one patch.
> +Running this script will remove all existing relations and replace them with
> the relations found in the file.
Please could you also wrap your lines as in the rest of the file.
> +
> +.. option:: infile
> +
> + input patch groups file.
> +
Lastly, when applying the patch, I see:
dja@dja-thinkpad ~/d/p/patchwork (master)> git pw patch apply 1321548 -s
.git/rebase-apply/patch:80: trailing whitespace.
.git/rebase-apply/patch:81: trailing whitespace.
def handle(self, *args, **options):
.git/rebase-apply/patch:98: trailing whitespace.
.git/rebase-apply/patch:102: trailing whitespace.
.git/rebase-apply/patch:122: trailing whitespace.
warning: squelched 4 whitespace errors
warning: 9 lines add whitespace errors.
Applying: management: introduce replacerelations command
Could you please fix those whitespace errors?
Kind regards,
Daniel
> rehash
> ~~~~~~
>
> diff --git a/patchwork/management/commands/replacerelations.py
> b/patchwork/management/commands/replacerelations.py
> new file mode 100644
> index 0000000..0d9581d
> --- /dev/null
> +++ b/patchwork/management/commands/replacerelations.py
> @@ -0,0 +1,72 @@
> +# Patchwork - automated patch tracking system
> +# Copyright (C) 2020 Rohit Sarkar <rohitsarkar5...@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/relations/patch-groups
> b/patchwork/tests/relations/patch-groups
> new file mode 100644
> index 0000000..593cf0f
> --- /dev/null
> +++ b/patchwork/tests/relations/patch-groups
> @@ -0,0 +1,3 @@
> +1 2
> +3 4 5
> +6
> diff --git a/patchwork/tests/relations/patch-groups-missing-patch-ids
> b/patchwork/tests/relations/patch-groups-missing-patch-ids
> new file mode 100644
> index 0000000..091b195
> --- /dev/null
> +++ b/patchwork/tests/relations/patch-groups-missing-patch-ids
> @@ -0,0 +1,4 @@
> +1 2
> +3 4 5 7
> +6 8
> +9 10
> diff --git a/patchwork/tests/test_management.py
> b/patchwork/tests/test_management.py
> index 66c6bad..c648cc7 100644
> --- a/patchwork/tests/test_management.py
> +++ b/patchwork/tests/test_management.py
> @@ -11,7 +11,7 @@ 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 +124,28 @@ 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', stdout=out)
> + self.assertEqual(exc.exception.code, 1)
> +
> + def test_valid_relations(self):
> + utils.create_patches(6)
> + out = StringIO()
> + call_command('replacerelations',
> + os.path.join(TEST_RELATIONS_DIR,
> + 'patch-groups'),
> + stdout=out)
> +
> + self.assertEqual(models.PatchRelation.objects.count(), 2)
> +
> + call_command('replacerelations',
> + os.path.join(TEST_RELATIONS_DIR,
> + 'patch-groups-missing-patch-ids'),
> + stdout=out)
> +
> + self.assertEqual(models.PatchRelation.objects.count(), 2)
> +
> --
> 2.23.0.385.gbc12974a89
_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
