| Message ID | 20200720104223.6957-2-rohitsarkar5398@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add replacerelations management command | expand |
Hi Rohit,
Some more things:
1) `docker-compose run web --tox` fails, both pep8 and in some of the
substantive tests:
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
.............................................................................................................................................................................................................................................................................Invalid path: xyz123random
.F..........................................................................................................................................................................................................................................s................................................................................
======================================================================
FAIL: test_valid_relations (patchwork.tests.test_management.ReplacerelationsTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/patchwork/patchwork/patchwork/tests/test_management.py", line 143, in test_valid_relations
self.assertEqual(models.PatchRelation.objects.count(), 2)
AssertionError: 0 != 2
----------------------------------------------------------------------
(Please could you also silence the 'invalid path' print during tests?)
2) There's an line at the end of test_management.py.
3) I reformatted the docs to wrap at 80 columns and I made some minor
changes to the wording and punctuation in the process. Please wrap
future changes at 80 columns. I also found that the patch groups file
doesn't render as separate lines in a web browser, it renders as
"1 3 5 2 7 10 11 12"
I've fixed this.
You can render the docs to html locally with
docker-compose run web --tox -e docs
Here's my proposed docs:
diff --git a/docs/deployment/management.rst b/docs/deployment/management.rst
index 9c57f1962283..7bff699329fb 100644
--- a/docs/deployment/management.rst
+++ b/docs/deployment/management.rst
@@ -116,6 +116,42 @@ 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
~~~~~~
Kind regards,
Daniel
> 1) `docker-compose run web --tox` fails, both pep8 and in some of the > substantive tests: > > Creating test database for alias 'default'... > System check identified no issues (0 silenced). > .............................................................................................................................................................................................................................................................................Invalid path: xyz123random > .F..........................................................................................................................................................................................................................................s................................................................................ I think this might be because by the time you get to your test, a number of patches have been created and destroyed, so the patches are no longer numbered the way you expect. I didn't pick this up before because last time I ran the tests there was only the one with the invalid path. > 3) I reformatted the docs to wrap at 80 columns and I made some minor > changes to the wording and punctuation in the process. Please wrap > future changes at 80 columns. I also found that the patch groups file > doesn't render as separate lines in a web browser, it renders as I screwed up the suggested docs, try this one instead. Kind regards, Daniel diff --git a/docs/deployment/management.rst b/docs/deployment/management.rst index 9c57f1962283..dcee9ff518f6 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 ~~~~~~
Hey, On Thu, Jul 23, 2020 at 11:00:08AM +1000, Daniel Axtens wrote: > > 1) `docker-compose run web --tox` fails, both pep8 and in some of the > > substantive tests: > > > > Creating test database for alias 'default'... > > System check identified no issues (0 silenced). > > .............................................................................................................................................................................................................................................................................Invalid path: xyz123random > > .F..........................................................................................................................................................................................................................................s................................................................................ > > I think this might be because by the time you get to your test, a number > of patches have been created and destroyed, so the patches are no longer > numbered the way you expect. I didn't pick this up before because last > time I ran the tests there was only the one with the invalid path. Yeah, I missed this as I wasnt running the entire suite but just the individual test. Was wondering if there is any best practice for this for writing tests that depend on the ids in the database? Thanks, Rohit
Rohit Sarkar <rohitsarkar5398@gmail.com> writes: > Hey, > On Thu, Jul 23, 2020 at 11:00:08AM +1000, Daniel Axtens wrote: >> > 1) `docker-compose run web --tox` fails, both pep8 and in some of the >> > substantive tests: >> > >> > Creating test database for alias 'default'... >> > System check identified no issues (0 silenced). >> > .............................................................................................................................................................................................................................................................................Invalid path: xyz123random >> > .F..........................................................................................................................................................................................................................................s................................................................................ >> >> I think this might be because by the time you get to your test, a number >> of patches have been created and destroyed, so the patches are no longer >> numbered the way you expect. I didn't pick this up before because last >> time I ran the tests there was only the one with the invalid path. > > Yeah, I missed this as I wasnt running the entire suite but just the > individual test. Was wondering if there is any best practice for this > for writing tests that depend on the ids in the database? Not sure what best practice would be. I think in your case, you'll need to create your patches, extract their IDs, and then write those into a temporary file (see the tempfile module). Kind regards, Daniel > > Thanks, > Rohit
Hey, Thanks for all the patient feedback Daniel. Have sent out a v5 addressing these issues. Thanks, Rohit
diff --git a/docs/deployment/management.rst b/docs/deployment/management.rst index 9c57f19..1437550 100644 --- a/docs/deployment/management.rst +++ b/docs/deployment/management.rst @@ -116,6 +116,34 @@ 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 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 patches. +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/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..905369d 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) +
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 | 28 ++++++++ .../management/commands/replacerelations.py | 72 +++++++++++++++++++ patchwork/tests/__init__.py | 1 + patchwork/tests/relations/patch-groups | 3 + .../relations/patch-groups-missing-patch-ids | 4 ++ patchwork/tests/test_management.py | 27 ++++++- 6 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 patchwork/management/commands/replacerelations.py create mode 100644 patchwork/tests/relations/patch-groups create mode 100644 patchwork/tests/relations/patch-groups-missing-patch-ids