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

[Brian Bouterse]

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-conve
>>>> ntions/#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/20180608/23f99200/attachment.htm>