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 <ttere...@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 <davidda...@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.agu...@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 < >>> ttere...@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@redhat.com >>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>> >>> _______________________________________________ >>> Pulp-dev mailing list >>> Pulp-dev@redhat.com >>> https://www.redhat.com/mailman/listinfo/pulp-dev >>> >> _______________________________________________ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev >
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
