I think we should just rename id -> _id and leave it at that. It's the only one that is immediately problematic and seems to have the highest chance of collision.
If we also renamed last_updated -> _last_updated, it would be right next to "last_synced" and "last_published" and would look inconsistent. _id would look fine because we also have _href. @Jeff, What do you think? On Mon, Jun 11, 2018 at 9:13 AM, Brian Bouterse <bbout...@redhat.com> wrote: > @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-conve >>>>>>> ntions/#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
