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

[Andy Doan]

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).

> 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.

>> +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).