[Pulp-dev] 'id' versus 'pulp_id' on Content
bbouters at redhat.com
Fri May 25 16:15:22 UTC 2018
I wrote up a Redmine issue to make this change here:
https://pulp.plan.io/issues/3704 Please look at it and groom it if it
@jortel I especially want to make sure you're comfortable with this change.
If anyone is -1 on this change please reply saying so.
On Thu, May 24, 2018 at 5:06 PM, Brian Bouterse <bbouters at redhat.com> wrote:
> +1 to using _id, _created, and _last_updated only on MasterModel. It looks
> like leading underscores for field names are fine:
> https://stackoverflow.com/a/25509372 That will resolve this issue. Also
> everyone can still use .pk instead of ._id because Django automatically
> makes a .pk attribute on every model that also acts as the primary key
> regardless of the primary key's name. Also I don't think we have this issue
> elsewhere, only because Content is so heavily subclassed so I don't think
> we'll do leading _ to field names in other places.
> @jortel I don't understand what you wrote; maybe try explaining it some
> more? The field "id" a plugin writer would define would contain the data
> they are obligated to hold for that content type. They didn't select the
> name "id"; it's the name the designers of that content type selected when
> they modeled that content type. So the designers of Errata itself chose
> Errata.id and what data should go inside there.
> On Wed, May 23, 2018 at 10:33 AM, Jeff Ortel <jortel at redhat.com> wrote:
>> 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. Every
>> field in base models used by plugins has potential for name collisions.
>> Where does it end? Every column having a pulp_ or _ prefix? Plugins
>> create relatively few tables and it doesn't seem unreasonable for plugin
>> writers to select other names to resolve naming conflicts.
>> On 05/23/2018 07:50 AM, Brian Bouterse wrote:
>> Currently the Content model  has 'id' as it's primary key which is
>> inherited from MasterModel here . By naming our pk 'id', we are
>> preventing plugin writers from also using that field. That field name is
>> common for content types. For example: both RPM and Nuget content also
>> expect to use the 'id' field to store data about the content type itself
>> (not Pulp's pk). We learned about the Nuget incompatibility at
>> ConfigMgmgtCamp from a community member. I learned about this issue with
>> RPM from @dalley.
>> The only workaround a plugin writer has is to call their field 'rpm_id'
>> or something like that. I don't see how it's unavoidable that this renaming
>> won't be passed directly onto the user for things like filtering, creating
>> units, etc. I think that is an undesirable outcome just so that the Pulp pk
>> can be named 'id'.
>> One option would be to rename 'id' to 'pulp_id' at the MasterModel. This
>> is also somewhat ugly for Pulp developers, but it would be (a) crystal
>> clear to the user in all cases and (b) allow Content writers to model their
>> content types correctly.
>> Another option would be to rename the pk for 'Content' specifically and
>> not at the MasterModel level. I think that would create more confusion than
>> benefit so I recommend doing it at the MasterModel level.
>> What do you all think?
>> : https://github.com/pulp/pulp/blob/6f492ee8fac94b8562dc62d87e
>> : https://github.com/pulp/pulp/blob/d1dc089890f167617fe9917af0
>> Pulp-dev mailing listPulp-dev at redhat.comhttps://www.redhat.com/mailman/listinfo/pulp-dev
>> Pulp-dev mailing list
>> Pulp-dev at redhat.com
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Pulp-dev