[RFC 07/11] REST: Add Patch Checks to the API

[Finucane, Stephen]

On 10 May 17:30, Andy Doan wrote:
> On 05/09/2016 08:58 AM, Finucane, Stephen wrote:
> >On 15 Apr 13:24, Andy Doan wrote:
> 
> >>+
> >>+    def test_create(self):
> >>+        """Ensure creations can be performed by user of patch."""
> >>+        check = {
> >>+            'state': Check.STATE_SUCCESS,
> >
> >I think we should expose the string value here, rather than an integer
> >that people have to go an convert. I'm pretty sure this is what happens
> >with the XML-RPC API, and if not the predecessor to this patch [1]
> >could be some help (I'll resend shortly).
> >
> >[1] https://patchwork.ozlabs.org/patch/602073/
> 
> Good idea. Fixed in rev2
> 
> >>+from rest_framework_nested.routers import NestedSimpleRouter
> >
> >How widely available is this dependency? Is it available in the Ubuntu
> >LTS repos?
> 
> Its not in Debian or Ubuntu (neither is DRF though).

OK. I was concerned one was distro-provided and the other not. If it's
all pip then that's fine.

> >Also, this is another dependency that you need to check for
> >as part of 'settings.py' (where you raise the RuntimeException if
> >missing).
> 
> I'm only raising a RuntimeError when there's a config mismatch:
> 
> if settings.ENABLE_REST_API:
>     if 'rest_framework' not in settings.INSTALLED_APPS:
>         raise RuntimeError(
>             'djangorestframework must be installed to enable the REST
> 
> ie - normal ImportErrors will bubble up if either are enabled but
> not installed.

Ah, yes. My mistake.

> >>+patches_router = NestedSimpleRouter(router, r'patches', lookup='patch')
> >>+patches_router.register(r'checks', ChecksViewSet, base_name='patch-checks')
> >>+patches_router.register(r'check', CheckViewSet, base_name='patch-check')
> >
> >I think that 'check' would be more valuable as part of the 'patch'
> >endpoint, rather than as a separate endpoint. The whole idea of 'check'
> >is to provide a quick overview of the status, before delving into more
> >detailed results via 'checks'.
> 
> The problem with putting "check" into the Patch endpoint is that we
> have an explosion of DB hits (self.check_set.all() for each Patch).

Had a similar issue with the patch list screen: that was resolved using
'prefetch_related' [1]. Seems like it's possible to do the same thing
with DRF [2][3]?

Stephen

[1] https://github.com/getpatchwork/patchwork/commit/2d65dc28
[2] http://stackoverflow.com/a/16212442/613428
[3] https://github.com/tomchristie/django-rest-framework/issues/1964#issuecomment-59523476