Message ID | 20200414062102.6798-3-dja@axtens.net |
---|---|
State | Accepted |
Headers | show |
Series | v2.2 fixups for OzLabs | expand |
Hi Daniel, > In the process of fixing the previous bug, I realised that: > > a) /api/patches/msgid is a perfectly reasonable thing to attempt > b) We have no way of finding a patch by message id in the API > > We can't actualy make /api/patches/msgid work because it may not > be unique, but we can add a filter. LGTM: https://patchwork.ozlabs.org/api/patches/?msgid=20200414062102.6798-3-dja@axtens.net Tested-by: Jeremy Kerr <jk@ozlabs.org> Cheers, Jeremy
On 14/4/20 4:21 pm, Daniel Axtens wrote: > In the process of fixing the previous bug, I realised that: > > a) /api/patches/msgid is a perfectly reasonable thing to attempt > b) We have no way of finding a patch by message id in the API > > We can't actualy make /api/patches/msgid work because it may not > be unique, but we can add a filter. > > I'm shoehorning this into stable/2.2, even though it's technically > an API change: it's minor, not incompatible and in hindsight a > glaring hole. > > Cc: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Daniel Axtens <dja@axtens.net> This is indeed something we would expect to be able to do in the API now that we support lookup by message ID for the web. Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> > --- > docs/api/schemas/latest/patchwork.yaml | 16 ++++++++++++++++ > docs/api/schemas/patchwork.j2 | 18 ++++++++++++++++++ > docs/api/schemas/v1.2/patchwork.yaml | 16 ++++++++++++++++ > patchwork/api/filters.py | 14 ++++++++++---- > patchwork/tests/api/test_cover.py | 12 ++++++++++++ > patchwork/tests/api/test_patch.py | 12 ++++++++++++ > .../rest-filter-msgid-41f693cd4e53cf93.yaml | 6 ++++++ > 7 files changed, 90 insertions(+), 4 deletions(-) > create mode 100644 releasenotes/notes/rest-filter-msgid-41f693cd4e53cf93.yaml > > diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml > index 13cdc9cd78fd..cc0d97e696b6 100644 > --- a/docs/api/schemas/latest/patchwork.yaml > +++ b/docs/api/schemas/latest/patchwork.yaml > @@ -246,6 +246,14 @@ paths: > schema: > title: '' > type: string > + - in: query > + name: msgid > + description: > > + The cover message-id as a case-sensitive string, without leading or > + trailing angle brackets, to filter by. > + schema: > + title: '' > + type: string > responses: > '200': > description: '' > @@ -474,6 +482,14 @@ paths: > schema: > title: '' > type: string > + - in: query > + name: msgid > + description: > > + The patch message-id as a case-sensitive string, without leading or > + trailing angle brackets, to filter by. > + schema: > + title: '' > + type: string > responses: > '200': > description: '' > diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 > index bd714d5e7a2a..f5618d41faa0 100644 > --- a/docs/api/schemas/patchwork.j2 > +++ b/docs/api/schemas/patchwork.j2 > @@ -251,6 +251,16 @@ paths: > schema: > title: '' > type: string > +{% if version >= (1, 2) %} > + - in: query > + name: msgid > + description: > > + The cover message-id as a case-sensitive string, without leading or > + trailing angle brackets, to filter by. > + schema: > + title: '' > + type: string > +{% endif %} > responses: > '200': > description: '' > @@ -488,6 +498,14 @@ paths: > schema: > title: '' > type: string > + - in: query > + name: msgid > + description: > > + The patch message-id as a case-sensitive string, without leading or > + trailing angle brackets, to filter by. > + schema: > + title: '' > + type: string > {% endif %} > responses: > '200': > diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml > index db2ed122eec2..7bdbe66997c0 100644 > --- a/docs/api/schemas/v1.2/patchwork.yaml > +++ b/docs/api/schemas/v1.2/patchwork.yaml > @@ -246,6 +246,14 @@ paths: > schema: > title: '' > type: string > + - in: query > + name: msgid > + description: > > + The cover message-id as a case-sensitive string, without leading or > + trailing angle brackets, to filter by. > + schema: > + title: '' > + type: string > responses: > '200': > description: '' > @@ -474,6 +482,14 @@ paths: > schema: > title: '' > type: string > + - in: query > + name: msgid > + description: > > + The patch message-id as a case-sensitive string, without leading or > + trailing angle brackets, to filter by. > + schema: > + title: '' > + type: string > responses: > '200': > description: '' > diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py > index deb5ace11880..93e6281bf5e6 100644 > --- a/patchwork/api/filters.py > +++ b/patchwork/api/filters.py > @@ -184,6 +184,10 @@ class SeriesFilterSet(TimestampMixin, BaseFilterSet): > fields = ('submitter', 'project') > > > +def msgid_filter(queryset, name, value): > + return queryset.filter(**{name: '<' + value + '>'}) > + > + > class CoverLetterFilterSet(TimestampMixin, BaseFilterSet): > > project = ProjectFilter(queryset=Project.objects.all(), distinct=False) > @@ -192,6 +196,7 @@ class CoverLetterFilterSet(TimestampMixin, BaseFilterSet): > series = BaseFilter(queryset=Project.objects.all(), > widget=MultipleHiddenInput, distinct=False) > submitter = PersonFilter(queryset=Person.objects.all(), distinct=False) > + msgid = CharFilter(method=msgid_filter) > > class Meta: > model = CoverLetter > @@ -210,17 +215,18 @@ class PatchFilterSet(TimestampMixin, BaseFilterSet): > delegate = UserFilter(queryset=User.objects.all(), distinct=False) > state = StateFilter(queryset=State.objects.all(), distinct=False) > hash = CharFilter(lookup_expr='iexact') > + msgid = CharFilter(method=msgid_filter) > > class Meta: > model = Patch > - # NOTE(dja): ideally we want to version the hash field, but I cannot > - # find a way to do that which is reliable and not extremely ugly. > + # NOTE(dja): ideally we want to version the hash/msgid field, but I > + # can't find a way to do that which is reliable and not extremely ugly. > # The best I can come up with is manually working with request.GET > # which seems to rather defeat the point of using django-filters. > fields = ('project', 'series', 'submitter', 'delegate', > - 'state', 'archived', 'hash') > + 'state', 'archived', 'hash', 'msgid') > versioned_fields = { > - '1.2': ('hash', ), > + '1.2': ('hash', 'msgid'), > } > > > diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py > index 5eeb1902e1d1..1b19ded1b4d5 100644 > --- a/patchwork/tests/api/test_cover.py > +++ b/patchwork/tests/api/test_cover.py > @@ -111,6 +111,18 @@ class TestCoverLetterAPI(utils.APITestCase): > 'submitter': 'test@example.org'}) > self.assertEqual(0, len(resp.data)) > > + def test_list_filter_msgid(self): > + """Filter covers by msgid.""" > + cover = create_cover() > + > + resp = self.client.get(self.api_url(), {'msgid': cover.url_msgid}) > + self.assertEqual([cover.id], [x['id'] for x in resp.data]) > + > + # empty response if nothing matches > + resp = self.client.get(self.api_url(), { > + 'msgid': 'fishfish@fish.fish'}) > + self.assertEqual(0, len(resp.data)) > + > @utils.store_samples('cover-list-1-0') > def test_list_version_1_0(self): > create_cover() > diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py > index b24c5ab28947..da2dd6e9084b 100644 > --- a/patchwork/tests/api/test_patch.py > +++ b/patchwork/tests/api/test_patch.py > @@ -199,6 +199,18 @@ class TestPatchAPI(utils.APITestCase): > {'hash': 'garbagevalue'}) > self.assertEqual(1, len(resp.data)) > > + def test_list_filter_msgid(self): > + """Filter patches by msgid.""" > + patch = self._create_patch() > + > + resp = self.client.get(self.api_url(), {'msgid': patch.url_msgid}) > + self.assertEqual([patch.id], [x['id'] for x in resp.data]) > + > + # empty response if nothing matches > + resp = self.client.get(self.api_url(), { > + 'msgid': 'fishfish@fish.fish'}) > + self.assertEqual(0, len(resp.data)) > + > @utils.store_samples('patch-list-1-0') > def test_list_version_1_0(self): > """List patches using API v1.0.""" > diff --git a/releasenotes/notes/rest-filter-msgid-41f693cd4e53cf93.yaml b/releasenotes/notes/rest-filter-msgid-41f693cd4e53cf93.yaml > new file mode 100644 > index 000000000000..0fcbbeb8a736 > --- /dev/null > +++ b/releasenotes/notes/rest-filter-msgid-41f693cd4e53cf93.yaml > @@ -0,0 +1,6 @@ > +--- > +api: > + - | > + The REST API now supports filtering patches and cover letters by message > + ID, using the ``msgid`` query parameter. Don't include leading or trailing > + angle brackets. >
On Tue, 2020-04-14 at 16:21 +1000, Daniel Axtens wrote: > In the process of fixing the previous bug, I realised that: > > a) /api/patches/msgid is a perfectly reasonable thing to attempt > b) We have no way of finding a patch by message id in the API > > We can't actualy make /api/patches/msgid work because it may not > be unique, but we can add a filter. > > I'm shoehorning this into stable/2.2, even though it's technically > an API change: it's minor, not incompatible and in hindsight a > glaring hole. > > Cc: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Daniel Axtens <dja@axtens.net> I meant to do this before cutting v2.2.0 and forgot. Probably should have had a list. It's not ideal putting this into an already released API version, but I agree it's a huge oversight. Let's make sure we get v2.2.1 out nice and quickly. Reviewed-by: Stephen Finucane <stephen@that.guru> Stephen
Stephen Finucane <stephen@that.guru> writes: > On Tue, 2020-04-14 at 16:21 +1000, Daniel Axtens wrote: >> In the process of fixing the previous bug, I realised that: >> >> a) /api/patches/msgid is a perfectly reasonable thing to attempt >> b) We have no way of finding a patch by message id in the API >> >> We can't actualy make /api/patches/msgid work because it may not >> be unique, but we can add a filter. >> >> I'm shoehorning this into stable/2.2, even though it's technically >> an API change: it's minor, not incompatible and in hindsight a >> glaring hole. >> >> Cc: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Daniel Axtens <dja@axtens.net> > > I meant to do this before cutting v2.2.0 and forgot. Probably should > have had a list. > > It's not ideal putting this into an already released API version, but I > agree it's a huge oversight. Let's make sure we get v2.2.1 out nice and > quickly. Indeed, doing this now. Thanks! Regards, Daniel > > Reviewed-by: Stephen Finucane <stephen@that.guru> > > Stephen
Daniel Axtens <dja@axtens.net> writes: > In the process of fixing the previous bug, I realised that: > > a) /api/patches/msgid is a perfectly reasonable thing to attempt > b) We have no way of finding a patch by message id in the API > > We can't actualy make /api/patches/msgid work because it may not > be unique, but we can add a filter. > > I'm shoehorning this into stable/2.2, even though it's technically > an API change: it's minor, not incompatible and in hindsight a > glaring hole. > > Cc: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Daniel Axtens <dja@axtens.net> Thanks, this is awesome. For reference here's my updated terrible script that I use to apply a series from a patch id or message id. cheers #!/usr/bin/python3 import sys import requests from subprocess import check_call session = requests.Session() arg = sys.argv[1] try: pid = int(arg) except ValueError: pid = None if pid is None: arg = arg.replace('id:', '') url = f'https://patchwork.ozlabs.org/api/patches/?msgid={arg}&project=linuxppc-dev' json = session.get(url).json()[0] else: url = f'https://patchwork.ozlabs.org/api/patches/{pid}/' json = session.get(url).json() sid = json['series'][0]['id'] cmd = f'git pw series apply {sid}' check_call(cmd.split()) print("OK")
diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml index 13cdc9cd78fd..cc0d97e696b6 100644 --- a/docs/api/schemas/latest/patchwork.yaml +++ b/docs/api/schemas/latest/patchwork.yaml @@ -246,6 +246,14 @@ paths: schema: title: '' type: string + - in: query + name: msgid + description: > + The cover message-id as a case-sensitive string, without leading or + trailing angle brackets, to filter by. + schema: + title: '' + type: string responses: '200': description: '' @@ -474,6 +482,14 @@ paths: schema: title: '' type: string + - in: query + name: msgid + description: > + The patch message-id as a case-sensitive string, without leading or + trailing angle brackets, to filter by. + schema: + title: '' + type: string responses: '200': description: '' diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 index bd714d5e7a2a..f5618d41faa0 100644 --- a/docs/api/schemas/patchwork.j2 +++ b/docs/api/schemas/patchwork.j2 @@ -251,6 +251,16 @@ paths: schema: title: '' type: string +{% if version >= (1, 2) %} + - in: query + name: msgid + description: > + The cover message-id as a case-sensitive string, without leading or + trailing angle brackets, to filter by. + schema: + title: '' + type: string +{% endif %} responses: '200': description: '' @@ -488,6 +498,14 @@ paths: schema: title: '' type: string + - in: query + name: msgid + description: > + The patch message-id as a case-sensitive string, without leading or + trailing angle brackets, to filter by. + schema: + title: '' + type: string {% endif %} responses: '200': diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml index db2ed122eec2..7bdbe66997c0 100644 --- a/docs/api/schemas/v1.2/patchwork.yaml +++ b/docs/api/schemas/v1.2/patchwork.yaml @@ -246,6 +246,14 @@ paths: schema: title: '' type: string + - in: query + name: msgid + description: > + The cover message-id as a case-sensitive string, without leading or + trailing angle brackets, to filter by. + schema: + title: '' + type: string responses: '200': description: '' @@ -474,6 +482,14 @@ paths: schema: title: '' type: string + - in: query + name: msgid + description: > + The patch message-id as a case-sensitive string, without leading or + trailing angle brackets, to filter by. + schema: + title: '' + type: string responses: '200': description: '' diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py index deb5ace11880..93e6281bf5e6 100644 --- a/patchwork/api/filters.py +++ b/patchwork/api/filters.py @@ -184,6 +184,10 @@ class SeriesFilterSet(TimestampMixin, BaseFilterSet): fields = ('submitter', 'project') +def msgid_filter(queryset, name, value): + return queryset.filter(**{name: '<' + value + '>'}) + + class CoverLetterFilterSet(TimestampMixin, BaseFilterSet): project = ProjectFilter(queryset=Project.objects.all(), distinct=False) @@ -192,6 +196,7 @@ class CoverLetterFilterSet(TimestampMixin, BaseFilterSet): series = BaseFilter(queryset=Project.objects.all(), widget=MultipleHiddenInput, distinct=False) submitter = PersonFilter(queryset=Person.objects.all(), distinct=False) + msgid = CharFilter(method=msgid_filter) class Meta: model = CoverLetter @@ -210,17 +215,18 @@ class PatchFilterSet(TimestampMixin, BaseFilterSet): delegate = UserFilter(queryset=User.objects.all(), distinct=False) state = StateFilter(queryset=State.objects.all(), distinct=False) hash = CharFilter(lookup_expr='iexact') + msgid = CharFilter(method=msgid_filter) class Meta: model = Patch - # NOTE(dja): ideally we want to version the hash field, but I cannot - # find a way to do that which is reliable and not extremely ugly. + # NOTE(dja): ideally we want to version the hash/msgid field, but I + # can't find a way to do that which is reliable and not extremely ugly. # The best I can come up with is manually working with request.GET # which seems to rather defeat the point of using django-filters. fields = ('project', 'series', 'submitter', 'delegate', - 'state', 'archived', 'hash') + 'state', 'archived', 'hash', 'msgid') versioned_fields = { - '1.2': ('hash', ), + '1.2': ('hash', 'msgid'), } diff --git a/patchwork/tests/api/test_cover.py b/patchwork/tests/api/test_cover.py index 5eeb1902e1d1..1b19ded1b4d5 100644 --- a/patchwork/tests/api/test_cover.py +++ b/patchwork/tests/api/test_cover.py @@ -111,6 +111,18 @@ class TestCoverLetterAPI(utils.APITestCase): 'submitter': 'test@example.org'}) self.assertEqual(0, len(resp.data)) + def test_list_filter_msgid(self): + """Filter covers by msgid.""" + cover = create_cover() + + resp = self.client.get(self.api_url(), {'msgid': cover.url_msgid}) + self.assertEqual([cover.id], [x['id'] for x in resp.data]) + + # empty response if nothing matches + resp = self.client.get(self.api_url(), { + 'msgid': 'fishfish@fish.fish'}) + self.assertEqual(0, len(resp.data)) + @utils.store_samples('cover-list-1-0') def test_list_version_1_0(self): create_cover() diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index b24c5ab28947..da2dd6e9084b 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -199,6 +199,18 @@ class TestPatchAPI(utils.APITestCase): {'hash': 'garbagevalue'}) self.assertEqual(1, len(resp.data)) + def test_list_filter_msgid(self): + """Filter patches by msgid.""" + patch = self._create_patch() + + resp = self.client.get(self.api_url(), {'msgid': patch.url_msgid}) + self.assertEqual([patch.id], [x['id'] for x in resp.data]) + + # empty response if nothing matches + resp = self.client.get(self.api_url(), { + 'msgid': 'fishfish@fish.fish'}) + self.assertEqual(0, len(resp.data)) + @utils.store_samples('patch-list-1-0') def test_list_version_1_0(self): """List patches using API v1.0.""" diff --git a/releasenotes/notes/rest-filter-msgid-41f693cd4e53cf93.yaml b/releasenotes/notes/rest-filter-msgid-41f693cd4e53cf93.yaml new file mode 100644 index 000000000000..0fcbbeb8a736 --- /dev/null +++ b/releasenotes/notes/rest-filter-msgid-41f693cd4e53cf93.yaml @@ -0,0 +1,6 @@ +--- +api: + - | + The REST API now supports filtering patches and cover letters by message + ID, using the ``msgid`` query parameter. Don't include leading or trailing + angle brackets.
In the process of fixing the previous bug, I realised that: a) /api/patches/msgid is a perfectly reasonable thing to attempt b) We have no way of finding a patch by message id in the API We can't actualy make /api/patches/msgid work because it may not be unique, but we can add a filter. I'm shoehorning this into stable/2.2, even though it's technically an API change: it's minor, not incompatible and in hindsight a glaring hole. Cc: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Daniel Axtens <dja@axtens.net> --- docs/api/schemas/latest/patchwork.yaml | 16 ++++++++++++++++ docs/api/schemas/patchwork.j2 | 18 ++++++++++++++++++ docs/api/schemas/v1.2/patchwork.yaml | 16 ++++++++++++++++ patchwork/api/filters.py | 14 ++++++++++---- patchwork/tests/api/test_cover.py | 12 ++++++++++++ patchwork/tests/api/test_patch.py | 12 ++++++++++++ .../rest-filter-msgid-41f693cd4e53cf93.yaml | 6 ++++++ 7 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/rest-filter-msgid-41f693cd4e53cf93.yaml