[Pulp-dev] Required fields for models at the DB level

[Brian Bouterse]

Thank you for bumping this thread.

The problem you illustrate with UpdateRecord makes sense and is
problematic. Django doesn't have Model.save() call full_clean() by default;
they document that here (
https://docs.djangoproject.com/en/dev/ref/models/instances/?from=olddocs#django.db.models.Model.full_clean
). I interpret the Django docs recommendation to this situation as: plugin
writers should either call full_clean() directly when handling model data
directly, or bake it into their save() methods.

pulpcore has a similar question w.r.t its models. Maybe pulpcore should be
calling full_clean() more, or baking the full_clean() call into save() in
its models? pulpcore and plugins should try to think about how this applies
to their code.

My opinion is that the impact is low enough that we don't need to make an
adjustment before the GA. I'd like to hear from anyone if you think there
is something we should do before the GA.



On Mon, Dec 2, 2019 at 10:20 AM Tatiana Tereshchenko <ttereshc at redhat.com>
wrote:

> Bump.
> Any thoughts?
>
> If we decide to change something, we'd better do it before GA, I think.
>
>
> On Mon, Nov 18, 2019 at 10:07 PM David Davis <daviddavis at redhat.com>
> wrote:
>
>> Without diving into the django code, I'm guessing that the problem is the
>> default value for text fields is a blank string. I bet if you set the
>> default=None on fields that don't accept null, they would become 'required'
>> (although I don't think this concept actually exists in django at the model
>> level).
>>
>> I think Fabricio's solution would work. However, it doesn't 'require'
>> fields so much as prevent blank strings from being saved in the database.
>> And I think that's what we want?
>>
>> David
>>
>>
>> On Mon, Nov 18, 2019 at 3:15 PM Fabricio Aguiar <
>> fabricio.aguiar at redhat.com> wrote:
>>
>>> the best way that I found so far is this:
>>> https://stackoverflow.com/a/56272674/5253051
>>> [image: image.png]4
>>> <https://stackoverflow.com/a/56272674/5253051>
>>>
>>> Best regards,
>>> Fabricio Aguiar
>>> Software Engineer, Pulp Project
>>> Red Hat Brazil - Latam <https://www.redhat.com/>
>>> +55 11 999652368
>>>
>>>
>>> On Mon, Nov 18, 2019 at 5:09 PM Tatiana Tereshchenko <
>>> ttereshc at redhat.com> wrote:
>>>
>>>> I noticed that there is no enforcement at the DB level to require
>>>> certain fields to be present on a model.
>>>>
>>>> I haven't checked all the field types but at least for TextField it's
>>>> seems to be true.
>>>> Even though `null` is False by default (`blank` as well), I can save a
>>>> model instance without most of fields set.
>>>>
>>>> As an example, for UpdateRecord [0] in RPM plugin, plugin writer can do
>>>> UpdateRecord().save() and it will be fine, all the fields are set to empty
>>>> string :/ It wouldn't be possible to save it twice but it's due to the
>>>> uniqueness constraints.
>>>>
>>>> In case plugin writer doesn't set properly all the required fields,
>>>> bad/corrupted model instances will be silently saved in the DB and plugin
>>>> can potentially have data migration issues later.
>>>>
>>>> Any suggestions how to handle that? Or do I miss anything?
>>>>
>>>> Tanya
>>>>
>>>> [0]
>>>> https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/models/advisory.py#L77-L93
>>>> _______________________________________________
>>>> Pulp-dev mailing list
>>>> Pulp-dev at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>
>>> _______________________________________________
>>> Pulp-dev mailing list
>>> Pulp-dev at redhat.com
>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>
>> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20191202/aa3b41b2/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image.png
Type: image/png
Size: 64987 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20191202/aa3b41b2/attachment.png>