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

Brian Bouterse bbouters at redhat.com
Fri Jun 8 19:57:01 UTC 2018

@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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180608/3863999a/attachment.htm>

More information about the Pulp-dev mailing list