@dalley that is correct for the Django behaviors I believe. With this change for the model plugin writers will subclass, it would have "pk" as reserved and "_id" since we define the primary key in the ancestor class they inherit from. It also would have "_created" and "_last_updated" reserved. This should cause minimal collisions with the plugin writer's defined fields.
On Sat, Jun 9, 2018 at 1:51 AM, Daniel Alley <dal...@redhat.com> wrote: > Django reserves "pk" as a direct attribute, which just maps to whichever > field is defined as the primary key, which by default is an "id" field. > > So "pk" is always reserved and "id" is reserved by default, unless you > overdid it by defining your own primary key field. > > On Fri, Jun 8, 2018, 4:57 PM Brian Bouterse <bbout...@redhat.com> wrote: > >> That's good testing. The info I got in #django said differently. Your >> test looks better. >> >> This means there is at least 1 field name that plugin writers just can't >> use, but the issue I've heard about was for 'id' not 'pk'. We are in >> control of that one, so I want to bring it back to that. This does clarify >> that .pk can always be used which should make renaming object.id to >> object._id even less of a pain since object.pk can always be used. +1 to >> the convention @dalley recommended. >> >> On Fri, Jun 8, 2018 at 4:13 PM, David Davis <davidda...@redhat.com> >> wrote: >> >>> I did a quick test and it looks like pk can't be used as a field name: >>> >>> pulp_app.HelloWorld.pk: (fields.E003) 'pk' is a reserved word that >>> cannot be used as a field name. >>> >>> This kind of leads me to believe that there are going to be certain >>> field names that plugin writers simply cannot use and trying to ameliorate >>> this situation isn’t going to work. >>> >>> >>> David >>> >>> On Fri, Jun 8, 2018 at 3:57 PM, Brian Bouterse <bbout...@redhat.com> >>> wrote: >>> >>>> @dalley: I agree we have practical reasons motivating the 3 field name >>>> changes, of which the most important is 'id' to '_id'. w.r.t using " >>>> object.pk", that could create similar problems. If a plugin writer >>>> defines a 'pk' field then core code using using 'object.pk' will cause >>>> core code to receive their attribute and not the primary key. I think >>>> overall the strategy I think to minimize collisions we should use >>>> 'object._id' directly. How does that sound? >>>> >>>> @jortel: We're blocked on your -1 vote expressed for 3704. We have >>>> practical plugin writer issues with the current state. Can you elaborate on >>>> why we shouldn't go forward with https://pulp.plan.io/issues/3704 >>>> >>>> On Thu, Jun 7, 2018 at 1:52 PM, Daniel Alley <dal...@redhat.com> wrote: >>>> >>>>> The article[1] you mentioned states that 'ID' *should* be used for >>>>>> the PK >>>>> >>>>> >>>>> It does say this, but it says that the reasons for doing that are >>>>> because id is "short, simple, and unambiguous", and that the reason you >>>>> shouldn't prefix is because "the extra prefix is redundant". I think it's >>>>> really good advice for the general case, but the reasoning is based in >>>>> practicality and not strong convention, and in our case we do have some >>>>> other practical reasons to not do this. >>>>> >>>>> I don't feel super strongly in either direction at this point. I >>>>> think my personal preference is to change "id" to something else, and use >>>>> a >>>>> convention of "object.pk" instead of "object.id". The "pk" attribute >>>>> maps to whatever the primary key if we use that, we don't need to care >>>>> what >>>>> the field is called. >>>>> >>>>> Re: Brian >>>>> >>>>> When encountered, what they expressed is "Why would Pulp reserve >>>>>> common field that I need to define on my subclass?" My feeling here is >>>>>> that >>>>>> we don't actually have a good reason. >>>>>> >>>>> >>>>> "id" is technically reserved (unless you override it) by Django, since >>>>> it is the default PK field. If they were to directly subclass >>>>> `django.db.models.Model` they would have the same problem. This is the >>>>> only reason I'm a little conflicted, otherwise I be 100% in favor of >>>>> changing it. >>>>> >>>>> >>>>> >>>>> On Tue, May 29, 2018 at 12:46 PM, Jeff Ortel <jor...@redhat.com> >>>>> wrote: >>>>> >>>>>> On 05/29/2018 08:24 AM, Brian Bouterse wrote: >>>>>> >>>>>> On Fri, May 25, 2018 at 7:39 PM, Dana Walker <dawal...@redhat.com> >>>>>> wrote: >>>>>> >>>>>>> I'm basically -1 for the reasons Jeff enumerated but if he is ok >>>>>>> with this, I'm happy to go ahead with it. >>>>>>> >>>>>>> [Jeff]: >>>>>>>> In classic relational modeling, using ID as the primary key is >>>>>>>> common practice. Especially when ORMs are involved. The "id" added by >>>>>>>> plugin writers is a natural key so naming it ID goes against >>>>>>>> convention. >>>>>>>> >>>>>>> >>>>>>> This is echoed here, for further reading (though perhaps this >>>>>>> article is overly simplified for our needs) in the sections "Key Fields" >>>>>>> and "Prefixes and Suffixes (are bad)": >>>>>>> https://launchbylunch.com/posts/2014/Feb/16/sql-naming-conventions/ >>>>>>> >>>>>>> >>>>>> >>>>>> That is true, but this article also talks about avoiding reserved >>>>>> words as well. I think we're hearing 'id' is a commonly reserved word for >>>>>> content types being modeled by plugin writers. >>>>>> >>>>>> >>>>>> The article[1] you mentioned states that 'ID' *should* be used for >>>>>> the PK which means it is inappropriate for natural key fields defined by >>>>>> plugin writers. The reserved words caution in the article are DDL/DML >>>>>> reserved words "Ex: Avoid using words like user, lock, or table." >>>>>> not reserved by plugins. >>>>>> >>>>>> [1] https://launchbylunch.com/posts/2014/Feb/16/sql-naming- >>>>>> conventions/#primary-keys >>>>>> >>>>>> _______________________________________________ >>>>>> 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