@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