[Pulp-dev] 3.0 Validation update

[Sean Myers]

On 04/11/2017 05:08 PM, Michael Hrivnak wrote:
> I like thinking about this as business logic. Data may be valid, but it may
> not be usable in a particular context.
> 
> To help figure out where such logic should live, it may help to think about
> where the check is most important. I've described that time as "at the time
> of use" earlier in this discussion (maybe just on IRC). With sync as an
> example, a workflow will load an importer's config from the database, check
> it for problems, and then immediately use the values it just inspected.
> This is the critical moment where it must gracefully handle unusable data.
> This check ensures correct behavior and avoids an unhandled exception or
> crash.

+1

> We can and should also check for problems at earlier opportunities, such as
> at the time a user tries to queue a sync. This improves the user
> experience, but it is not required for correct and safe operation.

I think this adds an extra error-checking step for integrators, so in one
way it improves the user experience, but by adding a step we're also taking
away from it.

> Given that, I think it makes sense to put the check close to the data. A
> method on the model seems reasonable. In terms of polluting the model with
> business logic, it isn't that different from defining a custom query set on
> the model, which django encourages.

+0, we've split off importers and publishers and, for now at least, put
the business logic in the plugin API's proxy model. It's +0 instead of +1
because the entire sync process is difficult to compare to adding a custom
queryset.

> As a slight tangent, some applications take this sort of checking even
> further. An admirable approach in REST API design, which may not be a good
> idea for us at this time but is interesting to note, is to make a behavior
> such as "sync" only available via a link accessed via a known name in an
> object's representation. That's a mouthful, so here's an example:
> 
> {
>   "id": "foo",
>   "feed": "http://cdn.redhat.com/stuff/",
>   "_links":
>   {
>     "self": "http://here.com/importers/foo",
>     "sync": "http://here.com/importers/foo/sync"
>   }
> }
> 
> Consider that the link for starting a sync is not part of the published
> API, except that it must be obtained from this representation. There are
> two advantages here.
> 
> <snip>

+1 I love this. It reminds me that the "content" view that I snuck onto
the repo ViewSet should probably live in a similar "_links" mapping.

> And to re-state, this kind of thing may not be worth our time to actually
> do right now, and I'm not proposing it. I don't know to what extent DRF
> would make this easy. But I wanted to bring it up for interest's sake as
> yet another place in the workflow, even closer to the end user than the
> other two we've discussed, where applications have an opportunity to
> utilize context checking of data.

I mentioned this just now in another email on this, but the "pre-validation"
idea seems like a great thing to punt to 3.y. For 3.0 I think it's
entirely appropriate to let the API be a tad austere, avoiding the expense
(for now) of tackling what has proven to be a complicated and time-consuming
concern.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 866 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20170411/d2941c07/attachment.sig>