[Pulp-dev] 'id' versus 'pulp_id' on Content

Mon Jun 11 13:13:24 UTC 2018

@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 <dalley at 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 <bbouters at 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 <daviddavis at 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 <bbouters at 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 <dalley at 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 <jortel at redhat.com>
>>>>> wrote:
>>>>>
>>>>>> On 05/29/2018 08:24 AM, Brian Bouterse wrote:
>>>>>>
>>>>>> On Fri, May 25, 2018 at 7:39 PM, Dana Walker <dawalker at 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 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/20180611/8c4076ee/attachment.htm>