| Message ID | 20170614181423.5570-1-aconole@bytheb.org |
|---|---|
| State | Accepted |
| Headers | show |
Aaron Conole <aconole@bytheb.org> writes: > This commit allows users of the REST API to query for events based on > the date field. This will allow utility writers to select a smaller > subset of events when polling. > > Signed-off-by: Aaron Conole <aconole@bytheb.org> > --- It should be noted that my motivation for this is to implement a git-pw event poll command for CI integration. If you think there's a better way of achieving this, let me know. -Aaron
Aaron Conole <aconole@bytheb.org> writes: > Aaron Conole <aconole@bytheb.org> writes: > >> This commit allows users of the REST API to query for events based on >> the date field. This will allow utility writers to select a smaller >> subset of events when polling. >> >> Signed-off-by: Aaron Conole <aconole@bytheb.org> >> --- > > It should be noted that my motivation for this is to implement a git-pw > event poll command for CI integration. > > If you think there's a better way of achieving this, let me know. You should check out Snowpatch: Andrew Donnellan and Russell Currey (on this list) are the developers. https://developer.ibm.com/open/openprojects/snowpatch/ I don't know how they do it but I'm sure they'd be happy to explain it at length :) Regards, Daniel > > -Aaron > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On 15/06/17 09:59, Daniel Axtens wrote: > Aaron Conole <aconole@bytheb.org> writes: > >> Aaron Conole <aconole@bytheb.org> writes: >> >>> This commit allows users of the REST API to query for events based on >>> the date field. This will allow utility writers to select a smaller >>> subset of events when polling. >>> >>> Signed-off-by: Aaron Conole <aconole@bytheb.org> >>> --- >> >> It should be noted that my motivation for this is to implement a git-pw >> event poll command for CI integration. >> >> If you think there's a better way of achieving this, let me know. > > You should check out Snowpatch: Andrew Donnellan and Russell Currey (on > this list) are the developers. > > https://developer.ibm.com/open/openprojects/snowpatch/ > > I don't know how they do it but I'm sure they'd be happy to explain it > at length :) We don't currently use the events API at all - we just poll the patches API at fixed intervals and look for patches that don't already have test results. We might change this approach in future.
On Wed, 2017-06-14 at 14:16 -0400, Aaron Conole wrote: > Aaron Conole <aconole@bytheb.org> writes: > > > This commit allows users of the REST API to query for events based on > > the date field. This will allow utility writers to select a smaller > > subset of events when polling. > > > > Signed-off-by: Aaron Conole <aconole@bytheb.org> I think I missed this initially because I'd intended to use cursor-based pagination [1] for '/events'. Cursor-based pagination would be beneficial in so far as it allows us to grab a pseudo-permalink to page of events, which isn't possible with page number-based pagination because the page numbers can change way to darn quick. However, it does make '/events' unique in how it does pagination. I can go either way with this, and can submit an RFC for this this evening, if it would help to compare the two. > > --- > > It should be noted that my motivation for this is to implement a git-pw > event poll command for CI integration. Hmm, so I'll admit I'm not overly fond of this idea. I'm trying to keep git-pw as a client side tool for git-patchwork interaction and not a catch-all application for all things Patchwork like pwclient currently is. "Create a check" doesn't sound like something Acme developer would be doing from within their Git repo. > If you think there's a better way of achieving this, let me know. Any chance you could keep this as a separate tool? I'd be happy to spin out some of the 'git-pw' code into a 'patchwork-api' library, if that would help (though there really isn't much to spin out). Alternatively, some of the folks here were working on a "snowpatch" tool. I've no idea how far along it is nor how it works, but it could be a good option if it's still progressing. Stephen PS: I published a (now rather outdated) outdated blog [2] and talk [3] on how to do basic interaction between the REST API and Jenkins a while back. Might be worth looking at if you haven't seen them already. [1] http://www.django-rest-framework.org/api-guide/pagination/#cursorpagination [2] https://that.guru/blog/patchwork-and-ci-in-a-tree/ [3] https://speakerdeck.com/stephenfin/mailing-list-meet-ci
Stephen Finucane <stephen@that.guru> writes: > On Wed, 2017-06-14 at 14:16 -0400, Aaron Conole wrote: >> Aaron Conole <aconole@bytheb.org> writes: >> >> > This commit allows users of the REST API to query for events based on >> > the date field. This will allow utility writers to select a smaller >> > subset of events when polling. >> > >> > Signed-off-by: Aaron Conole <aconole@bytheb.org> > > I think I missed this initially because I'd intended to use cursor-based > pagination [1] for '/events'. Cursor-based pagination would be beneficial in so > far as it allows us to grab a pseudo-permalink to page of events, which isn't > possible with page number-based pagination because the page numbers can change > way to darn quick. However, it does make '/events' unique in how it does > pagination. I can go either way with this, and can submit an RFC for this this > evening, if it would help to compare the two. Sure; I'm fairly inexperienced with web development, so I don't understand the nuance between the two approaches. >> > --- >> >> It should be noted that my motivation for this is to implement a git-pw >> event poll command for CI integration. > > Hmm, so I'll admit I'm not overly fond of this idea. I'm trying to keep git-pw > as a client side tool for git-patchwork interaction and not a catch-all > application for all things Patchwork like pwclient currently is. "Create a > check" doesn't sound like something Acme developer would be doing from within > their Git repo. Okay. I'm approaching this from the angle of "I want to do X, how do I achieve it today." :) I thought it made sense to have a running stream of events that I could poll on, especially since I envision the workflow like: 1. Wake up 2. Do human things 3. git pw events list --category=series-new* --since=<last time> 4. scroll through that work (ie: git pw series apply X, review, comment, etc. keep going until all are done) 5. go to 3. Maybe I'm not thinking about events correctly, though? Then I could basically make steps 3,4,5 be done by a machine instead, and have it run things like checkpatch, make check, etc. I'll look at the snowpatch tool, as well. Thanks. >> If you think there's a better way of achieving this, let me know. > > Any chance you could keep this as a separate tool? I'd be happy to spin out > some of the 'git-pw' code into a 'patchwork-api' library, if that would help > (though there really isn't much to spin out). Alternatively, some of the folks > here were working on a "snowpatch" tool. I've no idea how far along it is nor > how it works, but it could be a good option if it's still progressing. Sure thing; it doesn't have to be part of git-pw; I was looking at the details from the freedesktop fork of patchwork [A] and it seemed like a useful functionality. As for spinning it out, that's probably a good idea. > Stephen > > PS: I published a (now rather outdated) outdated blog [2] and talk [3] on how > to do basic interaction between the REST API and Jenkins a while back. Might be > worth looking at if you haven't seen them already. > > [1] http://www.django-rest-framework.org/api-guide/pagination/#cursorpagination > [2] https://that.guru/blog/patchwork-and-ci-in-a-tree/ > [3] https://speakerdeck.com/stephenfin/mailing-list-meet-ci [A] http://patchwork-freedesktop.readthedocs.io/en/latest/testing.html
Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes: > On 15/06/17 09:59, Daniel Axtens wrote: >> Aaron Conole <aconole@bytheb.org> writes: >> >>> Aaron Conole <aconole@bytheb.org> writes: >>> >>>> This commit allows users of the REST API to query for events based on >>>> the date field. This will allow utility writers to select a smaller >>>> subset of events when polling. >>>> >>>> Signed-off-by: Aaron Conole <aconole@bytheb.org> >>>> --- >>> >>> It should be noted that my motivation for this is to implement a git-pw >>> event poll command for CI integration. >>> >>> If you think there's a better way of achieving this, let me know. >> >> You should check out Snowpatch: Andrew Donnellan and Russell Currey (on >> this list) are the developers. >> >> https://developer.ibm.com/open/openprojects/snowpatch/ >> >> I don't know how they do it but I'm sure they'd be happy to explain it >> at length :) > > We don't currently use the events API at all - we just poll the > patches API at fixed intervals and look for patches that don't already > have test results. We might change this approach in future. Thanks for the pointers. I'll check this out.
On Thu, 2017-06-15 at 09:57 -0400, Aaron Conole wrote: > Stephen Finucane <stephen@that.guru> writes: > > > On Wed, 2017-06-14 at 14:16 -0400, Aaron Conole wrote: > > > Aaron Conole <aconole@bytheb.org> writes: > > > > > > > This commit allows users of the REST API to query for events based on > > > > the date field. This will allow utility writers to select a smaller > > > > subset of events when polling. > > > > > > > > Signed-off-by: Aaron Conole <aconole@bytheb.org> > > > > I think I missed this initially because I'd intended to use cursor-based > > pagination [1] for '/events'. Cursor-based pagination would be beneficial > > in so > > far as it allows us to grab a pseudo-permalink to page of events, which > > isn't > > possible with page number-based pagination because the page numbers can > > change > > way to darn quick. However, it does make '/events' unique in how it does > > pagination. I can go either way with this, and can submit an RFC for this > > this > > evening, if it would help to compare the two. > > Sure; I'm fairly inexperienced with web development, so I don't > understand the nuance between the two approaches. Sorry for the delay. I've just submitted that RFC now. To compare the two, I'd suggest you take a look at the below: https://www.sitepoint.com/paginating-real-time-data-cursor-based-pagination/ The gist is that the high volume of events makes it likely that stuff will get lost between pages. This really makes sense for '/events' and it might even make sense for '/patches' and '/series' (as they're also sorted in chronological order by default), though I doubt we'll change those now. > > > > --- > > > > > > It should be noted that my motivation for this is to implement a git-pw > > > event poll command for CI integration. > > > > Hmm, so I'll admit I'm not overly fond of this idea. I'm trying to keep > > git-pw > > as a client side tool for git-patchwork interaction and not a catch-all > > application for all things Patchwork like pwclient currently is. "Create a > > check" doesn't sound like something Acme developer would be doing from > > within > > their Git repo. > > Okay. I'm approaching this from the angle of "I want to do X, how do I > achieve it today." :) I thought it made sense to have a running stream > of events that I could poll on, especially since I envision the workflow > like: > > 1. Wake up > 2. Do human things > 3. git pw events list --category=series-new* --since=<last time> > 4. scroll through that work (ie: git pw series apply X, review, comment, > etc. keep going until all are done) > 5. go to 3. Actually, that makes sense. I'd be game to accept a PR for this. What I wouldn't be a fan of is integrating checks and I thought this is what you were suggesting. > Maybe I'm not thinking about events correctly, though? > > Then I could basically make steps 3,4,5 be done by a machine instead, > and have it run things like checkpatch, make check, etc. I'll look at > the snowpatch tool, as well. Thanks. Yeah, anything that can be automated should be. Machines, however, shouldn't be using git-pw - they should make their own calls or use some kind of as-yet-non- existent Patchwork REST library. Cheers, Stephen
On Wed, 2017-06-21 at 21:08 +0100, Stephen Finucane wrote: > On Thu, 2017-06-15 at 09:57 -0400, Aaron Conole wrote: > > Stephen Finucane <stephen@that.guru> writes: > > > > > On Wed, 2017-06-14 at 14:16 -0400, Aaron Conole wrote: > > > > Aaron Conole <aconole@bytheb.org> writes: > > > > > > > > > This commit allows users of the REST API to query for events based on > > > > > the date field. This will allow utility writers to select a smaller > > > > > subset of events when polling. > > > > > > > > > > Signed-off-by: Aaron Conole <aconole@bytheb.org> > > > > > > I think I missed this initially because I'd intended to use cursor-based > > > pagination [1] for '/events'. Cursor-based pagination would be beneficial > > > in so > > > far as it allows us to grab a pseudo-permalink to page of events, which > > > isn't > > > possible with page number-based pagination because the page numbers can > > > change > > > way to darn quick. However, it does make '/events' unique in how it does > > > pagination. I can go either way with this, and can submit an RFC for this > > > this > > > evening, if it would help to compare the two. > > > > Sure; I'm fairly inexperienced with web development, so I don't > > understand the nuance between the two approaches. > > Sorry for the delay. I've just submitted that RFC now. > > To compare the two, I'd suggest you take a look at the below: > > https://www.sitepoint.com/paginating-real-time-data-cursor-based-pagination > / > > The gist is that the high volume of events makes it likely that stuff will > get > lost between pages. This really makes sense for '/events' and it might even > make sense for '/patches' and '/series' (as they're also sorted in > chronological order by default), though I doubt we'll change those now. ...then again, we could accomplish the same thing with the 'until' filter. So long as someone specifies this, we shouldn't see anything getting entered in between the pages. I think we can ignore my RFC and merge this, if you agree? Stephen
On Wed, 2017-06-14 at 14:14 -0400, Aaron Conole wrote: > This commit allows users of the REST API to query for events based on > the date field. This will allow utility writers to select a smaller > subset of events when polling. > > Signed-off-by: Aaron Conole <aconole@bytheb.org> Reviewed-by: Stephen Finucane <stephen@that.guru> ...and applied. Thanks, Aaron. Stephen
diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py index 3dc215c..666a3d8 100644 --- a/patchwork/api/filters.py +++ b/patchwork/api/filters.py @@ -127,7 +127,7 @@ class CheckFilter(TimestampMixin, FilterSet): fields = ('user', 'state', 'context') -class EventFilter(ProjectMixin, FilterSet): +class EventFilter(ProjectMixin, TimestampMixin, FilterSet): class Meta: model = Event
This commit allows users of the REST API to query for events based on the date field. This will allow utility writers to select a smaller subset of events when polling. Signed-off-by: Aaron Conole <aconole@bytheb.org> --- patchwork/api/filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)