<div dir="ltr"><div>Where should business logic live? As an example, I want to consider the sync task [0] and the need to ensure that an importer is syncable. For now, let's say that an importer is syncable if it has a feed_url. <br></div><div><br></div><div>Since the call to sync an importer comes from the API, the earliest we can check that the configuration is syncable is in a not-yet-written SyncView. Checking syncability here is ideal because it allows us to immediately return a meaningful http response instead of happily returning information about a Task that we already know is doomed to fail. <br></div><div><br></div><div>Performing the check in the API layer is not enough. We have discussed edge cases that lead to an importer's feed_url being removed while the sync task is in the WAITING state. To make an assertion to the plugin that the feed_url exists, we have to check syncability again when the Task moves into an active state.</div><div><br></div><div>My thinking is that we should put this business logic on the model.</div><div><br></div><div>Admittedly, it is not a clean fit with the separation of concerns philosophy but we have already violated the concept by putting the sync method on the model. If sync is on the model, it seems like ensure_syncable should be too. </div><div><br></div><div>If we write the platform API layer to use this kind of business logic, then the plugins can add double checking business logic without modifying the API and Task layers.</div><div><br></div><div><br></div><div>[0]: <a href="https://pulp.plan.io/issues/2399">https://pulp.plan.io/issues/2399</a><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 7, 2017 at 2:14 PM, Sean Myers <span dir="ltr"><<a href="mailto:sean.myers@redhat.com" target="_blank">sean.myers@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 04/07/2017 12:08 PM, Brian Bouterse wrote:<br>
> == questions ==<br>
> * Where should ^ terms be documented?<br>
<br>
</span>I'm not really sure, but recommend the wiki as a good starting point for<br>
putting information that we should probably "officially" document *somewhere*,<br>
but at the moment we aren't quite sure where.<br>
<br>
<a href="https://pulp.plan.io/projects/pulp/wiki/Pulp_3_Developer_Notes" rel="noreferrer" target="_blank">https://pulp.plan.io/projects/<wbr>pulp/wiki/Pulp_3_Developer_<wbr>Notes</a><br>
<span class=""><br>
> * Take the case of a sync which has overrides provided? This isn't in the<br>
> MVP, but in the future it could be. In that case, does the serializer<br>
> associated with the importer validate the database data with the overrides<br>
> "added" on top of it?<br>
<br>
</span>My question here is "validate against what?". It makes sense to validate<br>
against database data, but as long as the overrides aren't themselves stored<br>
in the database, what does this really stop?<br>
<br>
For example, what prevents two simultaneous syncs of repos using overrides<br>
that would trigger a constraint violation if they were saved, but don't do<br>
this because we don't save the overrides?<br>
<span class=""><br>
> * For plugin writers writing a serializer for a subclassed Importer, do<br>
> they also need to express validations for fields defined on the base<br>
> Importer?<br>
<br>
</span>It depends on the validation. If it's just validating an individual field,<br>
no. If it's validation of a combination of values in multiple fields, and<br>
one of those fields in this case was defined in a subclass, the subclass<br>
will need to add the proper validation support.<br>
<span class=""><br>
> * The database still rejects data that doesn't adhere to the data layer<br>
> definition right? That occurs even without the DRF serializer correct?<br>
<br>
</span>Again, this depends. For example, attempting to store an int in a charfield<br>
will work, because Django will coerce that int to string on save. Attempting<br>
to store a string in an IntegerField will fail, though, because Django is<br>
not able to coerce str to int prior to saving. Generally, though, your<br>
understanding is correct. Anything that the database can't handle will be<br>
rejected.<br>
<span class=""><br>
> * In cases where data is created in the backend, do we need to validate<br>
> that as a general practice? If we do, do we call the DRF serializer<br>
> regularly in the backend or just let the database reject "bad" data at the<br>
> db level?<br>
<br>
</span>As a general practice, I don't think so. Specifically, though, when we're<br>
passing data around, like when a bit of platform code is taking incoming<br>
plugin data and passing it into some standard workflow that platform provides<br>
(like running sync on an importer, say) I think it's going to be a good idea<br>
and an all-around gentlemenly thing to do to validate that data in some way<br>
that appropriate to the process/workflow being invoked.<br>
<br>
I'm concerned about finding the balance between making things user-friendly<br>
for plugin writers and having our checking code that provides that user-<br>
friendly-ness itself be difficult to maintain and end up being pulp-developer-<br>
unfriendly.<br>
<br>
<br>______________________________<wbr>_________________<br>
Pulp-dev mailing list<br>
<a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>mailman/listinfo/pulp-dev</a><br>
<br></blockquote></div><br></div>