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

[Brian Bouterse]

Currently the Content model [0] has 'id' as it's primary key which is
inherited from MasterModel here [1]. 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?

[0]:
https://github.com/pulp/pulp/blob/6f492ee8fac94b8562dc62d87e6886869e052e7e/pulpcore/pulpcore/app/models/content.py#L106
[1]:
https://github.com/pulp/pulp/blob/d1dc089890f167617fe9917af087d5587708296b/pulpcore/pulpcore/app/models/base.py#L25

-Brian
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[David Davis]

Correct me if I’m wrong but don’t we call pk in most places instead of id?
If so, it would seem like replacing id with pulp_id wouldn’t be that ugly.

Also, I wonder about the created and last_updated fields. Seems like those
could cause conflicts in the future too. At the very least, it might be
nice to document which field names are reserved on the Content model.

David

On Wed, May 23, 2018 at 8:50 AM, Brian Bouterse  wrote:

> Currently the Content model [0] has 'id' as it's primary key which is
> inherited from MasterModel here [1]. 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?
>
> [0]: https://github.com/pulp/pulp/blob/6f492ee8fac94b8562dc62d87e
> 6886869e052e7e/pulpcore/pulpcore/app/models/content.py#L106
> [1]: https://github.com/pulp/pulp/blob/d1dc089890f167617fe9917af0
> 87d5587708296b/pulpcore/pulpcore/app/models/base.py#L25
>
> -Brian
>
> ___
> 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

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

[Daniel Alley]

Maybe

_created
> _id
> _last_updated
>

?

I'm not sure whether we use pk or id more often, but we use both quite a
lot.

On Wed, May 23, 2018 at 9:22 AM, David Davis  wrote:

> Correct me if I’m wrong but don’t we call pk in most places instead of id?
> If so, it would seem like replacing id with pulp_id wouldn’t be that ugly.
>
> Also, I wonder about the created and last_updated fields. Seems like those
> could cause conflicts in the future too. At the very least, it might be
> nice to document which field names are reserved on the Content model.
>
> David
>
> On Wed, May 23, 2018 at 8:50 AM, Brian Bouterse 
> wrote:
>
>> Currently the Content model [0] has 'id' as it's primary key which is
>> inherited from MasterModel here [1]. 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?
>>
>> [0]: https://github.com/pulp/pulp/blob/6f492ee8fac94b8562dc62d87e
>> 6886869e052e7e/pulpcore/pulpcore/app/models/content.py#L106
>> [1]: https://github.com/pulp/pulp/blob/d1dc089890f167617fe9917af0
>> 87d5587708296b/pulpcore/pulpcore/app/models/base.py#L25
>>
>> -Brian
>>
>> ___
>> 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

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

[Austin Macdonald]

In Pulp 2, having id fields bit us really badly. The reason may have been
specific to Mongoengine, but my understanding is that it is bad practice
anyway.

On Wed, May 23, 2018 at 9:22 AM, David Davis  wrote:

> Correct me if I’m wrong but don’t we call pk in most places instead of id?
> If so, it would seem like replacing id with pulp_id wouldn’t be that ugly.
>
> Also, I wonder about the created and last_updated fields. Seems like those
> could cause conflicts in the future too. At the very least, it might be
> nice to document which field names are reserved on the Content model.
>
> David
>
> On Wed, May 23, 2018 at 8:50 AM, Brian Bouterse 
> wrote:
>
>> Currently the Content model [0] has 'id' as it's primary key which is
>> inherited from MasterModel here [1]. 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?
>>
>> [0]: https://github.com/pulp/pulp/blob/6f492ee8fac94b8562dc62d87e
>> 6886869e052e7e/pulpcore/pulpcore/app/models/content.py#L106
>> [1]: https://github.com/pulp/pulp/blob/d1dc089890f167617fe9917af0
>> 87d5587708296b/pulpcore/pulpcore/app/models/base.py#L25
>>
>> -Brian
>>
>> ___
>> 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

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

[Jeff Ortel]

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 [0] has 'id' as it's primary key which is 
inherited from MasterModel here [1]. 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?

[0]: 
https://github.com/pulp/pulp/blob/6f492ee8fac94b8562dc62d87e6886869e052e7e/pulpcore/pulpcore/app/models/content.py#L106
[1]: 
https://github.com/pulp/pulp/blob/d1dc089890f167617fe9917af087d5587708296b/pulpcore/pulpcore/app/models/base.py#L25


-Brian


___
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

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

[Brian Bouterse]

+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  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 [0] has 'id' as it's primary key which is
> inherited from MasterModel here [1]. 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?
>
> [0]: https://github.com/pulp/pulp/blob/6f492ee8fac94b8562dc62d87e6886
> 869e052e7e/pulpcore/pulpcore/app/models/content.py#L106
> [1]: https://github.com/pulp/pulp/blob/d1dc089890f167617fe9917af087d5
> 587708296b/pulpcore/pulpcore/app/models/base.py#L25
>
> -Brian
>
>
> ___
> Pulp-dev mailing 
> listPulp-dev@redhat.comhttps://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

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

[Brian Bouterse]

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
looks ready.

@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  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  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 [0] has 'id' as it's primary key which is
>> inherited from MasterModel here [1]. 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?
>>
>> [0]: https://github.com/pulp/pulp/blob/6f492ee8fac94b8562dc62d87e
>> 6886869e052e7e/pulpcore/pulpcore/app/models/content.py#L106
>> [1]: https://github.com/pulp/pulp/blob/d1dc089890f167617fe9917af0
>> 87d5587708296b/pulpcore/pulpcore/app/models/base.py#L25
>>
>> -Brian
>>
>>
>> ___
>> Pulp-dev mailing 
>> listPulp-dev@redhat.comhttps://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

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

[Dana Walker]

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/

Plugin writers could use a surrogate key instead (unless I am
misunderstanding your example on Errata, Brian):
http://www.vertabelo.com/blog/technical-articles/natural-and-surrogate-primary-keys

[Jeff]:
> Every field in base models used by plugins has potential for name
> collisions.  Where does it end?  Every column having a pulp_ or _ prefix?
>

Exactly.  How far should we obfuscate our code base to prevent namespace
collisions in derivative work?  As David already pointed out, if we want to
do this for id, then maybe also for created and for last_updated.  Will we
want to continue on to adding pulp or _ before everything in Pulp (no)?  It
seems unnecessary.  So are we sure it's needed in these three cases?

[Austin]:
> In Pulp 2, having id fields bit us really badly. The reason may have been
> specific to Mongoengine, but my understanding is that it is bad practice
> anyway.
>

Now, I will give the caveat that there's no "one true way" to the naming
conventions, there are proponents for pulp (app) prefix, _ prefix, or
leaving the id alone, and as we already saw with the yaml file issue
recently, sometimes it's better to break a convention than to create bigger
problems elsewhere.  If we really were burned by the id fields in Pulp 2,
that's a valid enough reason to change approaches in Pulp 3.

Can anyone elaborate on what these problems were?  Was it related to
MongoDB being non-relational and not something we're likely to run into on
relational PostgreSQL?

[David]:
> At the very least, it might be nice to document which field names are
> reserved on the Content model.
>

+1 on this.  We want to encourage plugin writers by making things easy for
them, but there are relatively few instances where they would need to do a
workaround here, so documentation may suffice.  Either way we go, we should
document reserved names clearly for plugin writers to be aware of potential
namespacing issues (you know? just document the heck out of everything :P).

And on that note...happy weekend folks!

--Dana


Dana Walker

Associate Software Engineer

Red Hat




On Fri, May 25, 2018 at 12:15 PM, Brian Bouterse 
wrote:

> 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
> looks ready.
>
> @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 
> 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  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 [0] has 'id' as it's primary key which is
>>> inherited from MasterModel here [1]. By naming our pk 'id', we are
>>> preventing plugin writers from also using that field. That field name is
>>> common for content t

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

[Brian Bouterse]

We can't go forward if there are -1s, especially from core devs. That is
unfortunate because in the roughly 4-5 plugins being developed, 2 have
raised concerns that they can't model their content as they expect to. I
don't believe documentation is a viable resolution (see inline), so can the
blocking votes talk about what they would like to do instead?

On Fri, May 25, 2018 at 7:39 PM, Dana Walker  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.


> Plugin writers could use a surrogate key instead (unless I am
> misunderstanding your example on Errata, Brian):
> http://www.vertabelo.com/blog/technical-articles/natural-
> and-surrogate-primary-keys
>

I'm not seeing how this is related. This article questions natural vs
surrogate primary keys, but we've already adopted surrogate keys. We know
the primary key is a UUID (not a natural key) and it will never change. The
question I see here is different: what do we name the surrogate pk?


>
>
[Jeff]:
>> Every field in base models used by plugins has potential for name
>> collisions.  Where does it end?  Every column having a pulp_ or _ prefix?
>>
>
> Exactly.  How far should we obfuscate our code base to prevent namespace
> collisions in derivative work?  As David already pointed out, if we want to
> do this for id, then maybe also for created and for last_updated.  Will we
> want to continue on to adding pulp or _ before everything in Pulp (no)?  It
> seems unnecessary.  So are we sure it's needed in these three cases?
>

It feels like a slippery slope, but it's scoped. We probably define 50 -
100 fields across all of our models. This change is being proposed for 3
only. The reason it's right for these three is that these fields inherit
onto the Content model, and that is the model plugin writers subclass and
define lots and lots of fields on. The other areas of the plugin API either
aren't subclassed, or not subclassed with a large numbers of fields being
defined.


>
> [Austin]:
>> In Pulp 2, having id fields bit us really badly. The reason may have been
>> specific to Mongoengine, but my understanding is that it is bad practice
>> anyway.
>>
>
> Now, I will give the caveat that there's no "one true way" to the naming
> conventions, there are proponents for pulp (app) prefix, _ prefix, or
> leaving the id alone, and as we already saw with the yaml file issue
> recently, sometimes it's better to break a convention than to create bigger
> problems elsewhere.  If we really were burned by the id fields in Pulp 2,
> that's a valid enough reason to change approaches in Pulp 3.
>
> Can anyone elaborate on what these problems were?  Was it related to
> MongoDB being non-relational and not something we're likely to run into on
> relational PostgreSQL?
>
> [David]:
>> At the very least, it might be nice to document which field names are
>> reserved on the Content model.
>>
>
> +1 on this.  We want to encourage plugin writers by making things easy for
> them, but there are relatively few instances where they would need to do a
> workaround here, so documentation may suffice.  Either way we go, we should
> document reserved names clearly for plugin writers to be aware of potential
> namespacing issues (you know? just document the heck out of everything :P).
>

I don't think documentation is a viable resolution here. Both of the plugin
writers I've seen experience this, knew right away what the problem was.
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.


> And on that note...happy weekend folks!
>
> --Dana
>
>
> Dana Walker
>
> Associate Software Engineer
>
> Red Hat
>
> 
> 
>
> On Fri, May 25, 2018 at 12:15 PM, Brian Bouterse 
> wrote:
>
>> 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
>> looks ready.
>>
>> @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 
>> wrote:
>>
>>> +1 to using _id, _created, and _last_updated only on MasterModel. It

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

[Jeff Ortel]

On 05/29/2018 08:24 AM, Brian Bouterse wrote:
On Fri, May 25, 2018 at 7:39 PM, Dana Walker > 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@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Daniel Alley]

>
> 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  wrote:

> On 05/29/2018 08:24 AM, Brian Bouterse wrote:
>
> On Fri, May 25, 2018 at 7:39 PM, Dana Walker  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@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

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

[Brian Bouterse]

@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  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  wrote:
>
>> On 05/29/2018 08:24 AM, Brian Bouterse wrote:
>>
>> On Fri, May 25, 2018 at 7:39 PM, Dana Walker  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

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

[David Davis]

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  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  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  wrote:
>>
>>> On 05/29/2018 08:24 AM, Brian Bouterse wrote:
>>>
>>> On Fri, May 25, 2018 at 7:39 PM, Dana Walker 
>>> 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
>
>
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

Re: [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  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 
> 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  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  wrote:
>>>
 On 05/29/2018 08:24 AM, Brian Bouterse wrote:

 On Fri, May 25, 2018 at 7:39 PM, Dana Walker 
 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
>>>
>>>
>>
>> _

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

[Daniel Alley]

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  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  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 
>> 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  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  wrote:

> On 05/29/2018 08:24 AM, Brian Bouterse wrote:
>
> On Fri, May 25, 2018 at 7:39 PM, Dana Walker 
> 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/post

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

[Brian Bouterse]

@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  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  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 
>> 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 
>>> 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  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 
> wrote:
>
>> On 05/29/2018 08:24 AM, Brian Bouterse wrote:
>>
>> On Fri, May 25, 2018 at 7:39 PM, Dana Walker 
>> 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 t

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

[Daniel Alley]

I think we should just rename id -> _id and leave it at that.  It's the
only one that is immediately problematic and seems to have the highest
chance of collision.

If we also renamed last_updated -> _last_updated, it would be right next to
"last_synced" and "last_published" and would look inconsistent.  _id would
look fine because we also have _href.

@Jeff, What do you think?

On Mon, Jun 11, 2018 at 9:13 AM, Brian Bouterse  wrote:

> @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  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  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 
>>> 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 
 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 
> 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 
>> wrote:
>>
>>> On 05/29/2018 08:24 AM, Brian Bouterse wrote:
>>>
>>> On Fri, May 25, 2018 at 7:39 PM, Dana Walker 
>>> 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.

 [Je

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

[Jeff Ortel]

On 06/08/2018 02:57 PM, Brian Bouterse wrote:


@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


The 'ID' column is reserved for the primary key and is inappropriate for 
natural keys.  This is well establish convention and best practice.  
Plugin writers specify natural keys.  Also, by introducing '_' prefix 
(or any prefix) means a table could have both 'ID' and '_ID' columns 
which is especially confusing since the 'ID' column would not be the 
primary key.


How does naming the natural key for an rpm as 'rpm_id' cause a 
significant problem for plugin writers?


@bmbouters: just curious, where does the rpm 'id' come from and how is 
it used differently than the NEVREA composite natural key.


___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Daniel Alley]

>
> just curious, where does the rpm 'id' come from and how is it used
> differently than the NEVREA composite natural key.

It's a part of Erratum, not the actual RPM content, so it's unrelated to
NVREA.  An example of an errata "id" would be "RHEA-2013:1777".

I agree with your point about '_id' and 'id' being confusing.  I don't
think having 'pulp_id' would be so bad, but if there's still strong
objection to that idea, then I am fine with just moving forwards as-is and
making sure that we clearly document what field names plugin writers cannot
use.

On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel  wrote:

> On 06/08/2018 02:57 PM, Brian Bouterse wrote:
>
>>
>> @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
>>
>
> The 'ID' column is reserved for the primary key and is inappropriate for
> natural keys.  This is well establish convention and best practice.  Plugin
> writers specify natural keys.  Also, by introducing '_' prefix (or any
> prefix) means a table could have both 'ID' and '_ID' columns which is
> especially confusing since the 'ID' column would not be the primary key.
>
> How does naming the natural key for an rpm as 'rpm_id' cause a significant
> problem for plugin writers?
>
> @bmbouters: just curious, where does the rpm 'id' come from and how is it
> used differently than the NEVREA composite natural key.
>
>
> ___
> 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

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

[Brian Bouterse]

Silly question, but could we just call our 'id' 'pk' instead? Since that is
a fully reserved value in Django for the primary key it seems clearest to
just use that? What about that?

On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel  wrote:

> On 06/08/2018 02:57 PM, Brian Bouterse wrote:
>
>>
>> @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
>>
>
> The 'ID' column is reserved for the primary key and is inappropriate for
> natural keys.  This is well establish convention and best practice.


I don't understand this reasoning. Earlier in the thread we discussed how
the sources recommending these conventions also mention that if we have a
practical reason or problem with that convention to do something
differently. We received complaints on this name about collisions so I
don't follow how we should still follow the convention.


> Plugin writers specify natural keys.  Also, by introducing '_' prefix (or
> any prefix) means a table could have both 'ID' and '_ID' columns which is
> especially confusing since the 'ID' column would not be the primary key.
>

We have two concepts here that are similar, so I think that problem is
mostly unrelated to this decision. For example, if we leave the names as-is
we have this problem only now it's named id and errata_id and in addition
we'll have the problems listed below.


> How does naming the natural key for an rpm as 'rpm_id' cause a significant
> problem for plugin writers?
>

It's a good question because it's the whole motivation for this change.
It's not an rpm, it's an erratum which doesn't have nevra like a package.
It's also the problem from another content type I heard about at Config
Management Camp.

It causes problems in two ways:

* plugin users (not writers) who are familiar with 'id' as part of the
erratum data type would then have to also understand this field name
renaming that Pulp arbitrarily introduces. This could get confusing when
the user submit a filter with id='ID-2115858' and they find nothing because
'id' is matching on the primary key not on the 'id' attribute of the errata
like they expect. Those users would also be Pulp users so they'll
understand that _id means the pk.

* plugins specifically may wrap other tools and now they have to maintain
mappings as well. This is specifically the case with errata which the data
model is design to be name-for-name identical to the createrepo_c interface


> @bmbouters: just curious, where does the rpm 'id' come from and how is it
> used differently than the NEVREA composite natural key.
>
>
> ___
> 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

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

[David Davis]

On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse  wrote:

> Silly question, but could we just call our 'id' 'pk' instead? Since that
> is a fully reserved value in Django for the primary key it seems clearest
> to just use that? What about that?
>

Are you recommending we rename the id field to pk in the database? I’m not
sure if that would work.


>
> On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel  wrote:
>
>> On 06/08/2018 02:57 PM, Brian Bouterse wrote:
>>
>>>
>>> @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
>>>
>>
>> The 'ID' column is reserved for the primary key and is inappropriate for
>> natural keys.  This is well establish convention and best practice.
>
>
> I don't understand this reasoning. Earlier in the thread we discussed how
> the sources recommending these conventions also mention that if we have a
> practical reason or problem with that convention to do something
> differently. We received complaints on this name about collisions so I
> don't follow how we should still follow the convention.
>
>
>> Plugin writers specify natural keys.  Also, by introducing '_' prefix (or
>> any prefix) means a table could have both 'ID' and '_ID' columns which is
>> especially confusing since the 'ID' column would not be the primary key.
>>
>
> We have two concepts here that are similar, so I think that problem is
> mostly unrelated to this decision. For example, if we leave the names as-is
> we have this problem only now it's named id and errata_id and in addition
> we'll have the problems listed below.
>
>
>> How does naming the natural key for an rpm as 'rpm_id' cause a
>> significant problem for plugin writers?
>>
>
> It's a good question because it's the whole motivation for this change.
> It's not an rpm, it's an erratum which doesn't have nevra like a package.
> It's also the problem from another content type I heard about at Config
> Management Camp.
>
> It causes problems in two ways:
>
> * plugin users (not writers) who are familiar with 'id' as part of the
> erratum data type would then have to also understand this field name
> renaming that Pulp arbitrarily introduces. This could get confusing when
> the user submit a filter with id='ID-2115858' and they find nothing because
> 'id' is matching on the primary key not on the 'id' attribute of the errata
> like they expect. Those users would also be Pulp users so they'll
> understand that _id means the pk.
>

By the same logic, if Pulp users know that id means pk, wouldn’t they
therefore understand that the id is not the erratum id?


>
> * plugins specifically may wrap other tools and now they have to maintain
> mappings as well. This is specifically the case with errata which the data
> model is design to be name-for-name identical to the createrepo_c interface
>
>
Mapping one field to another seems rather minor. Or am I missing something?


>
>> @bmbouters: just curious, where does the rpm 'id' come from and how is it
>> used differently than the NEVREA composite natural key.
>>
>>
>> ___
>> 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

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

[Dana Walker]

+1 to everything daviddavis just said.

Dana Walker

Associate Software Engineer

Red Hat




On Tue, Jun 12, 2018 at 5:11 PM, David Davis  wrote:

> On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse 
> wrote:
>
>> Silly question, but could we just call our 'id' 'pk' instead? Since that
>> is a fully reserved value in Django for the primary key it seems clearest
>> to just use that? What about that?
>>
>
> Are you recommending we rename the id field to pk in the database? I’m not
> sure if that would work.
>
>
>>
>> On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel  wrote:
>>
>>> On 06/08/2018 02:57 PM, Brian Bouterse wrote:
>>>

 @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

>>>
>>> The 'ID' column is reserved for the primary key and is inappropriate for
>>> natural keys.  This is well establish convention and best practice.
>>
>>
>> I don't understand this reasoning. Earlier in the thread we discussed how
>> the sources recommending these conventions also mention that if we have a
>> practical reason or problem with that convention to do something
>> differently. We received complaints on this name about collisions so I
>> don't follow how we should still follow the convention.
>>
>>
>>> Plugin writers specify natural keys.  Also, by introducing '_' prefix
>>> (or any prefix) means a table could have both 'ID' and '_ID' columns which
>>> is especially confusing since the 'ID' column would not be the primary key.
>>>
>>
>> We have two concepts here that are similar, so I think that problem is
>> mostly unrelated to this decision. For example, if we leave the names as-is
>> we have this problem only now it's named id and errata_id and in addition
>> we'll have the problems listed below.
>>
>>
>>> How does naming the natural key for an rpm as 'rpm_id' cause a
>>> significant problem for plugin writers?
>>>
>>
>> It's a good question because it's the whole motivation for this change.
>> It's not an rpm, it's an erratum which doesn't have nevra like a package.
>> It's also the problem from another content type I heard about at Config
>> Management Camp.
>>
>> It causes problems in two ways:
>>
>> * plugin users (not writers) who are familiar with 'id' as part of the
>> erratum data type would then have to also understand this field name
>> renaming that Pulp arbitrarily introduces. This could get confusing when
>> the user submit a filter with id='ID-2115858' and they find nothing because
>> 'id' is matching on the primary key not on the 'id' attribute of the errata
>> like they expect. Those users would also be Pulp users so they'll
>> understand that _id means the pk.
>>
>
> By the same logic, if Pulp users know that id means pk, wouldn’t they
> therefore understand that the id is not the erratum id?
>
>
>>
>> * plugins specifically may wrap other tools and now they have to maintain
>> mappings as well. This is specifically the case with errata which the data
>> model is design to be name-for-name identical to the createrepo_c interface
>>
>>
> Mapping one field to another seems rather minor. Or am I missing something?
>
>
>>
>>> @bmbouters: just curious, where does the rpm 'id' come from and how is
>>> it used differently than the NEVREA composite natural key.
>>>
>>>
>>> ___
>>> 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
>
>
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Brian Bouterse]

On Tue, Jun 12, 2018 at 5:11 PM, David Davis  wrote:

> On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse 
> wrote:
>
>> Silly question, but could we just call our 'id' 'pk' instead? Since that
>> is a fully reserved value in Django for the primary key it seems clearest
>> to just use that? What about that?
>>
>
> Are you recommending we rename the id field to pk in the database? I’m not
> sure if that would work.
>

I'm wondering if its possible yes. #django says it is but they've been
wrong before. I haven't had a chance to test it.


>
>
>>
>> On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel  wrote:
>>
>>> On 06/08/2018 02:57 PM, Brian Bouterse wrote:
>>>

 @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

>>>
>>> The 'ID' column is reserved for the primary key and is inappropriate for
>>> natural keys.  This is well establish convention and best practice.
>>
>>
>> I don't understand this reasoning. Earlier in the thread we discussed how
>> the sources recommending these conventions also mention that if we have a
>> practical reason or problem with that convention to do something
>> differently. We received complaints on this name about collisions so I
>> don't follow how we should still follow the convention.
>>
>>
>>> Plugin writers specify natural keys.  Also, by introducing '_' prefix
>>> (or any prefix) means a table could have both 'ID' and '_ID' columns which
>>> is especially confusing since the 'ID' column would not be the primary key.
>>>
>>
>> We have two concepts here that are similar, so I think that problem is
>> mostly unrelated to this decision. For example, if we leave the names as-is
>> we have this problem only now it's named id and errata_id and in addition
>> we'll have the problems listed below.
>>
>>
>>> How does naming the natural key for an rpm as 'rpm_id' cause a
>>> significant problem for plugin writers?
>>>
>>
>> It's a good question because it's the whole motivation for this change.
>> It's not an rpm, it's an erratum which doesn't have nevra like a package.
>> It's also the problem from another content type I heard about at Config
>> Management Camp.
>>
>> It causes problems in two ways:
>>
>> * plugin users (not writers) who are familiar with 'id' as part of the
>> erratum data type would then have to also understand this field name
>> renaming that Pulp arbitrarily introduces. This could get confusing when
>> the user submit a filter with id='ID-2115858' and they find nothing because
>> 'id' is matching on the primary key not on the 'id' attribute of the errata
>> like they expect. Those users would also be Pulp users so they'll
>> understand that _id means the pk.
>>
>
> By the same logic, if Pulp users know that id means pk, wouldn’t they
> therefore understand that the id is not the erratum id?
>

Yes by that logic they probably would know, but the actual errata field is
named 'id' so my it's more about a correctness problem than confusion. A
correctness problem that passes along to users. If we're going to have
confusing names, let's pick names that allow for alignment with the names
already chosen by content types which commonly do use 'id'. Plugin writer's
aren't in control of those names; they already are chosen by content types.


>
>
>>
>> * plugins specifically may wrap other tools and now they have to maintain
>> mappings as well. This is specifically the case with errata which the data
>> model is design to be name-for-name identical to the createrepo_c interface
>>
>>
> Mapping one field to another seems rather minor. Or am I missing something?
>

After 22 emails on this thread it feels like a mountain out of a molehill.
I don't mean to waste people's time and energy. The reason I continue to
advocate for it is because when two, independent plugin writers give
feedback suggesting change, even small change, we should adopt it. The
complexity is minor, but it's there. I've always believed minimizing
complexity has been our goal.


>
>
>>
>>> @bmbouters: just curious, where does the rpm 'id' come from and how is
>>> it used differently than the NEVREA composite natural key.
>>>
>>>
>>> ___
>>> 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

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

[David Davis]

I do think the most compelling case for renaming the field is having
feedback from plugin writers to do so and also the desire to reduce
complexity for plugin writers. Honestly, I am on the fence about renaming
the field.

Just to clarify, is anyone a hard -1 on renaming id?


David

On Tue, Jun 12, 2018 at 5:32 PM, Brian Bouterse  wrote:

>
>
> On Tue, Jun 12, 2018 at 5:11 PM, David Davis 
> wrote:
>
>> On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse 
>> wrote:
>>
>>> Silly question, but could we just call our 'id' 'pk' instead? Since that
>>> is a fully reserved value in Django for the primary key it seems clearest
>>> to just use that? What about that?
>>>
>>
>> Are you recommending we rename the id field to pk in the database? I’m
>> not sure if that would work.
>>
>
> I'm wondering if its possible yes. #django says it is but they've been
> wrong before. I haven't had a chance to test it.
>
>
>>
>>
>>>
>>> On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel  wrote:
>>>
 On 06/08/2018 02:57 PM, Brian Bouterse wrote:

>
> @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
>

 The 'ID' column is reserved for the primary key and is inappropriate
 for natural keys.  This is well establish convention and best practice.
>>>
>>>
>>> I don't understand this reasoning. Earlier in the thread we discussed
>>> how the sources recommending these conventions also mention that if we have
>>> a practical reason or problem with that convention to do something
>>> differently. We received complaints on this name about collisions so I
>>> don't follow how we should still follow the convention.
>>>
>>>
 Plugin writers specify natural keys.  Also, by introducing '_' prefix
 (or any prefix) means a table could have both 'ID' and '_ID' columns which
 is especially confusing since the 'ID' column would not be the primary key.

>>>
>>> We have two concepts here that are similar, so I think that problem is
>>> mostly unrelated to this decision. For example, if we leave the names as-is
>>> we have this problem only now it's named id and errata_id and in addition
>>> we'll have the problems listed below.
>>>
>>>
 How does naming the natural key for an rpm as 'rpm_id' cause a
 significant problem for plugin writers?

>>>
>>> It's a good question because it's the whole motivation for this change.
>>> It's not an rpm, it's an erratum which doesn't have nevra like a package.
>>> It's also the problem from another content type I heard about at Config
>>> Management Camp.
>>>
>>> It causes problems in two ways:
>>>
>>> * plugin users (not writers) who are familiar with 'id' as part of the
>>> erratum data type would then have to also understand this field name
>>> renaming that Pulp arbitrarily introduces. This could get confusing when
>>> the user submit a filter with id='ID-2115858' and they find nothing because
>>> 'id' is matching on the primary key not on the 'id' attribute of the errata
>>> like they expect. Those users would also be Pulp users so they'll
>>> understand that _id means the pk.
>>>
>>
>> By the same logic, if Pulp users know that id means pk, wouldn’t they
>> therefore understand that the id is not the erratum id?
>>
>
> Yes by that logic they probably would know, but the actual errata field is
> named 'id' so my it's more about a correctness problem than confusion. A
> correctness problem that passes along to users. If we're going to have
> confusing names, let's pick names that allow for alignment with the names
> already chosen by content types which commonly do use 'id'. Plugin writer's
> aren't in control of those names; they already are chosen by content types.
>
>
>>
>>
>>>
>>> * plugins specifically may wrap other tools and now they have to
>>> maintain mappings as well. This is specifically the case with errata which
>>> the data model is design to be name-for-name identical to the createrepo_c
>>> interface
>>>
>>>
>> Mapping one field to another seems rather minor. Or am I missing
>> something?
>>
>
> After 22 emails on this thread it feels like a mountain out of a molehill.
> I don't mean to waste people's time and energy. The reason I continue to
> advocate for it is because when two, independent plugin writers give
> feedback suggesting change, even small change, we should adopt it. The
> complexity is minor, but it's there. I've always believed minimizing
> complexity has been our goal.
>
>
>>
>>
>>>
 @bmbouters: just curious, where does the rpm 'id' come from and how is
 it used differently than the NEVREA composite natural key.


 ___
 Pulp-dev mailing list
 Pulp-dev@redhat.com
 https://www.redhat.com/mailman/listinfo/pulp-dev

>>>
>>>
>>> ___
>>> Pulp-

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

[Jeff Ortel]

On 06/12/2018 05:03 PM, David Davis wrote:
I do think the most compelling case for renaming the field is having 
feedback from plugin writers to do so and also the desire to reduce 
complexity for plugin writers. Honestly, I am on the fence about 
renaming the field.


Just to clarify, is anyone a hard -1 on renaming id?


-1




David

On Tue, Jun 12, 2018 at 5:32 PM, Brian Bouterse > wrote:




On Tue, Jun 12, 2018 at 5:11 PM, David Davis
mailto:davidda...@redhat.com>> wrote:

On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse
mailto:bbout...@redhat.com>> wrote:

Silly question, but could we just call our 'id' 'pk'
instead? Since that is a fully reserved value in Django
for the primary key it seems clearest to just use that?
What about that?


Are you recommending we rename the id field to pk in the
database? I’m not sure if that would work.


I'm wondering if its possible yes. #django says it is but they've
been wrong before. I haven't had a chance to test it.


On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel
mailto:jor...@redhat.com>> wrote:

On 06/08/2018 02:57 PM, Brian Bouterse wrote:


@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



The 'ID' column is reserved for the primary key and is
inappropriate for natural keys.  This is well
establish convention and best practice. 



I don't understand this reasoning. Earlier in the thread
we discussed how the sources recommending these
conventions also mention that if we have a practical
reason or problem with that convention to do something
differently. We received complaints on this name about
collisions so I don't follow how we should still follow
the convention.

Plugin writers specify natural keys.  Also, by
introducing '_' prefix (or any prefix) means a table
could have both 'ID' and '_ID' columns which is
especially confusing since the 'ID' column would not
be the primary key.


We have two concepts here that are similar, so I think
that problem is mostly unrelated to this decision. For
example, if we leave the names as-is we have this problem
only now it's named id and errata_id and in addition we'll
have the problems listed below.


How does naming the natural key for an rpm as 'rpm_id'
cause a significant problem for plugin writers?


It's a good question because it's the whole motivation for
this change. It's not an rpm, it's an erratum which
doesn't have nevra like a package. It's also the problem
from another content type I heard about at Config
Management Camp.

It causes problems in two ways:

* plugin users (not writers) who are familiar with 'id' as
part of the erratum data type would then have to also
understand this field name renaming that Pulp arbitrarily
introduces. This could get confusing when the user submit
a filter with id='ID-2115858' and they find nothing
because 'id' is matching on the primary key not on the
'id' attribute of the errata like they expect. Those users
would also be Pulp users so they'll understand that _id
means the pk.


By the same logic, if Pulp users know that id means pk,
wouldn’t they therefore understand that the id is not the
erratum id?


Yes by that logic they probably would know, but the actual errata
field is named 'id' so my it's more about a correctness problem
than confusion. A correctness problem that passes along to users.
If we're going to have confusing names, let's pick names that
allow for alignment with the names already chosen by content types
which commonly do use 'id'. Plugin writer's aren't in control of
those names; they already are chosen by content types.


* plugins specifically may wrap other tools and now they
have to maintain mappings as well. This is specifically
the case with errata which the data model is design to be
name-for-name identical to the createrepo_c interface


Mapping one field to another seems rather minor. Or am I
missing something?


After 22 emails on this thread it feels like a mountain out of a
  

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

[Brian Bouterse]

Jeff, can you elaborate more on your -1. I want to understand it. I'm
struggling to appreciate an "it's a convention" argument without sources
like an RFC or similar. I don't believe internet articles are credible
sources because any viewpoint can be validated by an internet post.

To recap my interests here, it's about being responsive to the community.
We ask plugin writers for feedback and from two independent plugin writers
(not me) we received feedback that this name wasn't ideal. I want us to be
responsive to that. It's not only because I think their technical feedback
is legit (albeit small), but also because it's our strategy during the
beta/RC of Pulp3 core is to make adjustments based on plugin writer
feedback. To receive feedback and choose to not follow the recommendation
they suggested feels like not the way I want to interact with plugin
writers. This is my main concern with not making a change in this area.

All the best,
Brian


On Wed, Jun 13, 2018 at 10:26 AM, Jeff Ortel  wrote:

>
>
> On 06/12/2018 05:03 PM, David Davis wrote:
>
> I do think the most compelling case for renaming the field is having
> feedback from plugin writers to do so and also the desire to reduce
> complexity for plugin writers. Honestly, I am on the fence about renaming
> the field.
>
> Just to clarify, is anyone a hard -1 on renaming id?
>
>
> -1
>
>
>
>
> David
>
> On Tue, Jun 12, 2018 at 5:32 PM, Brian Bouterse 
> wrote:
>
>>
>>
>> On Tue, Jun 12, 2018 at 5:11 PM, David Davis 
>> wrote:
>>
>>> On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse 
>>> wrote:
>>>
 Silly question, but could we just call our 'id' 'pk' instead? Since
 that is a fully reserved value in Django for the primary key it seems
 clearest to just use that? What about that?

>>>
>>> Are you recommending we rename the id field to pk in the database? I’m
>>> not sure if that would work.
>>>
>>
>> I'm wondering if its possible yes. #django says it is but they've been
>> wrong before. I haven't had a chance to test it.
>>
>>
>>>
>>>

 On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel  wrote:

> On 06/08/2018 02:57 PM, Brian Bouterse wrote:
>
>>
>> @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
>>
>
> The 'ID' column is reserved for the primary key and is inappropriate
> for natural keys.  This is well establish convention and best practice.


 I don't understand this reasoning. Earlier in the thread we discussed
 how the sources recommending these conventions also mention that if we have
 a practical reason or problem with that convention to do something
 differently. We received complaints on this name about collisions so I
 don't follow how we should still follow the convention.


> Plugin writers specify natural keys.  Also, by introducing '_' prefix
> (or any prefix) means a table could have both 'ID' and '_ID' columns which
> is especially confusing since the 'ID' column would not be the primary 
> key.
>

 We have two concepts here that are similar, so I think that problem is
 mostly unrelated to this decision. For example, if we leave the names as-is
 we have this problem only now it's named id and errata_id and in addition
 we'll have the problems listed below.


> How does naming the natural key for an rpm as 'rpm_id' cause a
> significant problem for plugin writers?
>

 It's a good question because it's the whole motivation for this change.
 It's not an rpm, it's an erratum which doesn't have nevra like a package.
 It's also the problem from another content type I heard about at Config
 Management Camp.

 It causes problems in two ways:

 * plugin users (not writers) who are familiar with 'id' as part of the
 erratum data type would then have to also understand this field name
 renaming that Pulp arbitrarily introduces. This could get confusing when
 the user submit a filter with id='ID-2115858' and they find nothing because
 'id' is matching on the primary key not on the 'id' attribute of the errata
 like they expect. Those users would also be Pulp users so they'll
 understand that _id means the pk.

>>>
>>> By the same logic, if Pulp users know that id means pk, wouldn’t they
>>> therefore understand that the id is not the erratum id?
>>>
>>
>> Yes by that logic they probably would know, but the actual errata field
>> is named 'id' so my it's more about a correctness problem than confusion. A
>> correctness problem that passes along to users. If we're going to have
>> confusing names, let's pick names that allow for alignment with the names
>> already chosen by content types which commonly do use 'id'. Plugin writer's
>> aren't in control of those nam

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

[Dana Walker]

I'm -1 still but I had a few questions.

1) If we do this, what happens when someone uses multiple plugins and both
of them want to use id as well?  Wouldn't it be better to have the core
application reserving it and *all* plugins doing some derivative name?

2) I'd really like to hear more from the actual plugin writers desiring the
change on if this is a really big deal or just a minor complaint and if it
is important to them, have them write a story detailing why.  Who is the
best person for me to ask?  Someone on Satellite?  RPM?  Is someone reading
this who is effected and could give further details?

Thanks,

--Dana

Dana Walker

Associate Software Engineer

Red Hat




On Thu, Jun 14, 2018 at 9:08 AM, Brian Bouterse  wrote:

> Jeff, can you elaborate more on your -1. I want to understand it. I'm
> struggling to appreciate an "it's a convention" argument without sources
> like an RFC or similar. I don't believe internet articles are credible
> sources because any viewpoint can be validated by an internet post.
>
> To recap my interests here, it's about being responsive to the community.
> We ask plugin writers for feedback and from two independent plugin writers
> (not me) we received feedback that this name wasn't ideal. I want us to be
> responsive to that. It's not only because I think their technical feedback
> is legit (albeit small), but also because it's our strategy during the
> beta/RC of Pulp3 core is to make adjustments based on plugin writer
> feedback. To receive feedback and choose to not follow the recommendation
> they suggested feels like not the way I want to interact with plugin
> writers. This is my main concern with not making a change in this area.
>
> All the best,
> Brian
>
>
> On Wed, Jun 13, 2018 at 10:26 AM, Jeff Ortel  wrote:
>
>>
>>
>> On 06/12/2018 05:03 PM, David Davis wrote:
>>
>> I do think the most compelling case for renaming the field is having
>> feedback from plugin writers to do so and also the desire to reduce
>> complexity for plugin writers. Honestly, I am on the fence about renaming
>> the field.
>>
>> Just to clarify, is anyone a hard -1 on renaming id?
>>
>>
>> -1
>>
>>
>>
>>
>> David
>>
>> On Tue, Jun 12, 2018 at 5:32 PM, Brian Bouterse 
>> wrote:
>>
>>>
>>>
>>> On Tue, Jun 12, 2018 at 5:11 PM, David Davis 
>>> wrote:
>>>
 On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse 
 wrote:

> Silly question, but could we just call our 'id' 'pk' instead? Since
> that is a fully reserved value in Django for the primary key it seems
> clearest to just use that? What about that?
>

 Are you recommending we rename the id field to pk in the database? I’m
 not sure if that would work.

>>>
>>> I'm wondering if its possible yes. #django says it is but they've been
>>> wrong before. I haven't had a chance to test it.
>>>
>>>


>
> On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel  wrote:
>
>> On 06/08/2018 02:57 PM, Brian Bouterse wrote:
>>
>>>
>>> @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
>>>
>>
>> The 'ID' column is reserved for the primary key and is inappropriate
>> for natural keys.  This is well establish convention and best practice.
>
>
> I don't understand this reasoning. Earlier in the thread we discussed
> how the sources recommending these conventions also mention that if we 
> have
> a practical reason or problem with that convention to do something
> differently. We received complaints on this name about collisions so I
> don't follow how we should still follow the convention.
>
>
>> Plugin writers specify natural keys.  Also, by introducing '_' prefix
>> (or any prefix) means a table could have both 'ID' and '_ID' columns 
>> which
>> is especially confusing since the 'ID' column would not be the primary 
>> key.
>>
>
> We have two concepts here that are similar, so I think that problem is
> mostly unrelated to this decision. For example, if we leave the names 
> as-is
> we have this problem only now it's named id and errata_id and in addition
> we'll have the problems listed below.
>
>
>> How does naming the natural key for an rpm as 'rpm_id' cause a
>> significant problem for plugin writers?
>>
>
> It's a good question because it's the whole motivation for this
> change. It's not an rpm, it's an erratum which doesn't have nevra like a
> package. It's also the problem from another content type I heard about at
> Config Management Camp.
>
> It causes problems in two ways:
>
> * plugin users (not writers) who are familiar with 'id' as part of the
> erratum data type would then have to also unders

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

[Dennis Kliban]

On Thu, Jun 14, 2018 at 10:28 AM, Dana Walker  wrote:

> I'm -1 still but I had a few questions.
>

Why are you -1?


>
> 1) If we do this, what happens when someone uses multiple plugins and both
> of them want to use id as well?  Wouldn't it be better to have the core
> application reserving it and *all* plugins doing some derivative name?
>

Multiple plugins can have a model with a field name that is the same as a
model in another plugin.

The problem only exists when a plugin's model inherits from a pulpcore
model and tries to use a field name that is already used by the model it's
inheriting from.


>
> 2) I'd really like to hear more from the actual plugin writers desiring
> the change on if this is a really big deal or just a minor complaint and if
> it is important to them, have them write a story detailing why.  Who is the
> best person for me to ask?  Someone on Satellite?  RPM?  Is someone reading
> this who is effected and could give further details?
>
>
This was discussed during our workshop in Ghent back in February. I THINK
Alexander and Simon were the two people that brought this up. I know Simon
reads/writes on this list, but i am not sure about Alexander.


> Thanks,
>
> --Dana
>
> Dana Walker
>
> Associate Software Engineer
>
> Red Hat
>
> 
> 
>
> On Thu, Jun 14, 2018 at 9:08 AM, Brian Bouterse 
> wrote:
>
>> Jeff, can you elaborate more on your -1. I want to understand it. I'm
>> struggling to appreciate an "it's a convention" argument without sources
>> like an RFC or similar. I don't believe internet articles are credible
>> sources because any viewpoint can be validated by an internet post.
>>
>> To recap my interests here, it's about being responsive to the community.
>> We ask plugin writers for feedback and from two independent plugin writers
>> (not me) we received feedback that this name wasn't ideal. I want us to be
>> responsive to that. It's not only because I think their technical feedback
>> is legit (albeit small), but also because it's our strategy during the
>> beta/RC of Pulp3 core is to make adjustments based on plugin writer
>> feedback. To receive feedback and choose to not follow the recommendation
>> they suggested feels like not the way I want to interact with plugin
>> writers. This is my main concern with not making a change in this area.
>>
>> All the best,
>> Brian
>>
>>
>> On Wed, Jun 13, 2018 at 10:26 AM, Jeff Ortel  wrote:
>>
>>>
>>>
>>> On 06/12/2018 05:03 PM, David Davis wrote:
>>>
>>> I do think the most compelling case for renaming the field is having
>>> feedback from plugin writers to do so and also the desire to reduce
>>> complexity for plugin writers. Honestly, I am on the fence about renaming
>>> the field.
>>>
>>> Just to clarify, is anyone a hard -1 on renaming id?
>>>
>>>
>>> -1
>>>
>>>
>>>
>>>
>>> David
>>>
>>> On Tue, Jun 12, 2018 at 5:32 PM, Brian Bouterse 
>>> wrote:
>>>


 On Tue, Jun 12, 2018 at 5:11 PM, David Davis 
 wrote:

> On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse 
> wrote:
>
>> Silly question, but could we just call our 'id' 'pk' instead? Since
>> that is a fully reserved value in Django for the primary key it seems
>> clearest to just use that? What about that?
>>
>
> Are you recommending we rename the id field to pk in the database? I’m
> not sure if that would work.
>

 I'm wondering if its possible yes. #django says it is but they've been
 wrong before. I haven't had a chance to test it.


>
>
>>
>> On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel 
>> wrote:
>>
>>> On 06/08/2018 02:57 PM, Brian Bouterse wrote:
>>>

 @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

>>>
>>> The 'ID' column is reserved for the primary key and is inappropriate
>>> for natural keys.  This is well establish convention and best practice.
>>
>>
>> I don't understand this reasoning. Earlier in the thread we discussed
>> how the sources recommending these conventions also mention that if we 
>> have
>> a practical reason or problem with that convention to do something
>> differently. We received complaints on this name about collisions so I
>> don't follow how we should still follow the convention.
>>
>>
>>> Plugin writers specify natural keys.  Also, by introducing '_'
>>> prefix (or any prefix) means a table could have both 'ID' and '_ID' 
>>> columns
>>> which is especially confusing since the 'ID' column would not be the
>>> primary key.
>>>
>>
>> We have two concepts here that are similar, so I think that problem
>> is mostly unrelated to this decision. For example, if we leave the names
>> as

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

[Daniel Alley]

>
> 1) If we do this, what happens when someone uses multiple plugins and both
> of them want to use id as well?  Wouldn't it be better to have the core
> application reserving it and *all* plugins doing some derivative name?
>

One plugin wouldn't affect another since it's namespaced by table - it's
just that since plugin models inherit from core models, plugin fields can
conflict with fields from core and vice-versa.

2) I'd really like to hear more from the actual plugin writers desiring the
> change on if this is a really big deal or just a minor complaint and if it
> is important to them, have them write a story detailing why.  Who is the
> best person for me to ask?  Someone on Satellite?  RPM?  Is someone reading
> this who is effected and could give further details?
>

I don't feel strongly about it.  I understand Jeff's opposition and I also
understand that the fact that two independent workshop attendees from the
community ran into the issue means it's a common  tripping point
 for plugin writers.  I don't
feel like it's a serious issue in the long run, but there's definitely some
friction there. Even though it is a convention, it's not one understood
well enough that it prevents plugin writers from being frustrated by it.

I don't think we should drag this conversation out too much longer, but
I'll bring up one new point.  Pulp is unique in the sense that users
subclassing Pulp models and adding their own arbitrary fields is even a
concern in the first place.  Most Django projects simply don't need to care
about this at all.  Does this justify breaking the "common" convention?

I will make one more suggestion.  What about naming "id" -> "uuid"?  This
carries the clear connotation that it is a unique identifier so it is less
likely to be confusing a la "id and _id", and is still less likely to have
a namespace conflict.



On Thu, Jun 14, 2018 at 10:28 AM, Dana Walker  wrote:

> I'm -1 still but I had a few questions.
>
> 1) If we do this, what happens when someone uses multiple plugins and both
> of them want to use id as well?  Wouldn't it be better to have the core
> application reserving it and *all* plugins doing some derivative name?
>
> 2) I'd really like to hear more from the actual plugin writers desiring
> the change on if this is a really big deal or just a minor complaint and if
> it is important to them, have them write a story detailing why.  Who is the
> best person for me to ask?  Someone on Satellite?  RPM?  Is someone reading
> this who is effected and could give further details?
>
> Thanks,
>
> --Dana
>
> Dana Walker
>
> Associate Software Engineer
>
> Red Hat
>
> 
> 
>
> On Thu, Jun 14, 2018 at 9:08 AM, Brian Bouterse 
> wrote:
>
>> Jeff, can you elaborate more on your -1. I want to understand it. I'm
>> struggling to appreciate an "it's a convention" argument without sources
>> like an RFC or similar. I don't believe internet articles are credible
>> sources because any viewpoint can be validated by an internet post.
>>
>> To recap my interests here, it's about being responsive to the community.
>> We ask plugin writers for feedback and from two independent plugin writers
>> (not me) we received feedback that this name wasn't ideal. I want us to be
>> responsive to that. It's not only because I think their technical feedback
>> is legit (albeit small), but also because it's our strategy during the
>> beta/RC of Pulp3 core is to make adjustments based on plugin writer
>> feedback. To receive feedback and choose to not follow the recommendation
>> they suggested feels like not the way I want to interact with plugin
>> writers. This is my main concern with not making a change in this area.
>>
>> All the best,
>> Brian
>>
>>
>> On Wed, Jun 13, 2018 at 10:26 AM, Jeff Ortel  wrote:
>>
>>>
>>>
>>> On 06/12/2018 05:03 PM, David Davis wrote:
>>>
>>> I do think the most compelling case for renaming the field is having
>>> feedback from plugin writers to do so and also the desire to reduce
>>> complexity for plugin writers. Honestly, I am on the fence about renaming
>>> the field.
>>>
>>> Just to clarify, is anyone a hard -1 on renaming id?
>>>
>>>
>>> -1
>>>
>>>
>>>
>>>
>>> David
>>>
>>> On Tue, Jun 12, 2018 at 5:32 PM, Brian Bouterse 
>>> wrote:
>>>


 On Tue, Jun 12, 2018 at 5:11 PM, David Davis 
 wrote:

> On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse 
> wrote:
>
>> Silly question, but could we just call our 'id' 'pk' instead? Since
>> that is a fully reserved value in Django for the primary key it seems
>> clearest to just use that? What about that?
>>
>
> Are you recommending we rename the id field to pk in the database? I’m
> not sure if that would work.
>

 I'm wondering if its possible yes. #django says it is but they've been
 wrong before. I haven't had a chance to test it.


>
>
>>
>> On 

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

[Jeff Ortel]

On 06/14/2018 10:37 AM, Daniel Alley wrote:
I will make one more suggestion.  What about naming "id" -> "uuid"?  
This carries the clear connotation that it is a unique identifier so 
it is less likely to be confusing a la "id and _id", and is still less 
likely to have a namespace conflict.


Appreciate the suggestion but this would only be marginally less confusing.

___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Simon Baatz]

My 2 cents (in my role as a user, not plugin writer): I think the most
important argument in the entire discussion is this (not sure who
said this):

>* plugin users (not writers) who are familiar with 'id' as part of the
>erratum data type would then have to also understand this field name
>renaming that Pulp arbitrarily introduces. This could get confusing
>when the user submit a filter with id='ID-2115858' and they find
>nothing because 'id' is matching on the primary key not on the 'id'
>attribute of the errata like they expect. Those users would also be
>Pulp users so they'll understand that _id means the pk.
> 
>By the same logic, if Pulp users know that id means pk, wouldn’t they
>therefore understand that the id is not the erratum id?
> 
>Yes by that logic they probably would know, but the actual errata field
>is named 'id' so my it's more about a correctness problem than
>confusion. A correctness problem that passes along to users. If we're
>going to have confusing names, let's pick names that allow for
>alignment with the names already chosen by content types which commonly
>do use 'id'. Plugin writer's aren't in control of those names; they
>already are chosen by content types.


Assuming that Pulp users are aware of a pk named 'id' is a strong
assumption.  If the user is just managing entire repositories and
searches content from time to time when troubleshooting (using a CLI
for example), she/he could not care less that there is a field called
"id" that is not what it seems to be.

I think the entire discussion is focused on plugin writers too much.
The user visible consequences of this decision are more important from
my point of view. 

The situation is not directly comparable, but I already had fun with
confusing id names [0] in the CLI.  I must have been rather annoyed
at the time, since I still remember ;-)


[0] https://www.redhat.com/archives/pulp-list/2016-March/msg00048.html

___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Jeff Ortel]

On 06/14/2018 08:08 AM, Brian Bouterse wrote:
Jeff, can you elaborate more on your -1. I want to understand it. I'm 
struggling to appreciate an "it's a convention" argument without 
sources like an RFC or similar. I don't believe internet articles are 
credible sources because any viewpoint can be validated by an internet 
post.


RFCs typically define standards not conventions. Agreed on internet 
articles being available to support most any viewpoint. FWIW, I didn't 
introduce the aforementioned article.  Conventions are typically 
establish through example.  IMHO, most articles, tutorials, textbooks, 
etc use ID (or TABLE_ID) for the primary key. Also, this convention has 
been applied on /every/ project I have worked on.




To recap my interests here, it's about being responsive to the 
community. We ask plugin writers for feedback and from two independent 
plugin writers (not me) we received feedback that this name wasn't 
ideal. I want us to be responsive to that. It's not only because I 
think their technical feedback is legit (albeit small), but also 
because it's our strategy during the beta/RC of Pulp3 core is to make 
adjustments based on plugin writer feedback. To receive feedback and 
choose to not follow the recommendation they suggested feels like not 
the way I want to interact with plugin writers. This is my main 
concern with not making a change in this area.


I am sensitive to plugin writer requests but changing the name of the 
primary key field for every table in the core because 2 plugin writers 
said that it "wasn't ideal" seems rash.  I'm not convinced that this is 
a correctness concern but rather a minor inconvenience for what seems 
like (so far) a small percentage of plugins.  Plugin writers will always 
need to contend with naming conflicts and I believe the plugin is the 
proper place to resolve them.  I also want to be responsive to feedback 
but I think it's reasonable for the answer to be "no" when the request 
is not in the best interest of the project as a whole.


___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Jeff Ortel]

Thanks for your comment, Simon.

This introduces a perspective that is helpful to the discussion.  
Filtering on an 'ID' natural key field (such as errata_ID) in a way that 
is intuitive to the user is a significant use case.


On 06/14/2018 12:32 PM, Simon Baatz wrote:

My 2 cents (in my role as a user, not plugin writer): I think the most
important argument in the entire discussion is this (not sure who
said this):


* plugin users (not writers) who are familiar with 'id' as part of the
erratum data type would then have to also understand this field name
renaming that Pulp arbitrarily introduces. This could get confusing
when the user submit a filter with id='ID-2115858' and they find
nothing because 'id' is matching on the primary key not on the 'id'
attribute of the errata like they expect. Those users would also be
Pulp users so they'll understand that _id means the pk.

By the same logic, if Pulp users know that id means pk, wouldn’t they
therefore understand that the id is not the erratum id?

Yes by that logic they probably would know, but the actual errata field
is named 'id' so my it's more about a correctness problem than
confusion. A correctness problem that passes along to users. If we're
going to have confusing names, let's pick names that allow for
alignment with the names already chosen by content types which commonly
do use 'id'. Plugin writer's aren't in control of those names; they
already are chosen by content types.


Assuming that Pulp users are aware of a pk named 'id' is a strong
assumption.  If the user is just managing entire repositories and
searches content from time to time when troubleshooting (using a CLI
for example), she/he could not care less that there is a field called
"id" that is not what it seems to be.

I think the entire discussion is focused on plugin writers too much.
The user visible consequences of this decision are more important from
my point of view.

The situation is not directly comparable, but I already had fun with
confusing id names [0] in the CLI.  I must have been rather annoyed
at the time, since I still remember ;-)


[0] https://www.redhat.com/archives/pulp-list/2016-March/msg00048.html

___
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

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

[Jeff Ortel]

On 06/14/2018 12:19 PM, Jeff Ortel wrote:



On 06/14/2018 10:37 AM, Daniel Alley wrote:
I will make one more suggestion.  What about naming "id" -> "uuid"?  
This carries the clear connotation that it is a unique identifier so 
it is less likely to be confusing a la "id and _id", and is still 
less likely to have a namespace conflict.


Appreciate the suggestion but this would only be marginally less 
confusing.


Reconsidering this suggestion for the reasons you outlined.

___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Milan Kovacik]

uuid will most likely hit the same issue one day.

I wonder whether we're not forcing (model) inheritance where
composition might have been a better fit; how about pulp core
providing some sort of a (meta) container object that:
* holds the real plugin content unit
* is used to provide the core functionality (versioned repos etc)...
* queries to the inside of the container will be namespaced; e.g
GET@.../errata/content/pulp_meta_last_updated__gt=yesterday&pulp__errata_id=foo

--
milan

On Thu, Jun 14, 2018 at 9:38 PM, Jeff Ortel  wrote:
>
>
> On 06/14/2018 12:19 PM, Jeff Ortel wrote:
>>
>>
>>
>> On 06/14/2018 10:37 AM, Daniel Alley wrote:
>>>
>>> I will make one more suggestion.  What about naming "id" -> "uuid"?  This
>>> carries the clear connotation that it is a unique identifier so it is less
>>> likely to be confusing a la "id and _id", and is still less likely to have a
>>> namespace conflict.
>>
>>
>> Appreciate the suggestion but this would only be marginally less
>> confusing.
>
>
> Reconsidering this suggestion for the reasons you outlined.
>
>
> ___
> 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

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

[Ina Panova]

uuid sounds like good compromise.




Regards,

Ina Panova
Software Engineer| Pulp| Red Hat Inc.

"Do not go where the path may lead,
 go instead where there is no path and leave a trail."

On Thu, Jun 14, 2018 at 9:38 PM, Jeff Ortel  wrote:

>
>
> On 06/14/2018 12:19 PM, Jeff Ortel wrote:
>
>>
>>
>> On 06/14/2018 10:37 AM, Daniel Alley wrote:
>>
>>> I will make one more suggestion.  What about naming "id" -> "uuid"?
>>> This carries the clear connotation that it is a unique identifier so it is
>>> less likely to be confusing a la "id and _id", and is still less likely to
>>> have a namespace conflict.
>>>
>>
>> Appreciate the suggestion but this would only be marginally less
>> confusing.
>>
>
> Reconsidering this suggestion for the reasons you outlined.
>
>
> ___
> 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

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

[Brian Bouterse]

Having a user focus made me realize that it would be useful if a user could
easily tell which attributes were common to all content units versus just
that one content unit. When scripting for instance that is really useful to
know. We could document the 5 attributes that platform provides, but when
there are 20+ attributes on a subclassed content unit the underscores would
provide an easy, consistent answer to this question. This is an additional
reason separate from the the issue that our content attribute names are
colliding (id at least for now). The underscore prefix would make
collisions highly unlikely also. This problem is only scoped to the Content
unit since that is the place where we expect a large number of subclassed
attributes.

For this reason I believe using the _ as the prefix will provide 2
benefits. I wrote them here on this ticket:
https://pulp.plan.io/issues/3704

I am still +1 on adopting those changes for those reasons. More feedback is
welcome given the additional problem statements and discussion.

On Mon, Jun 18, 2018 at 8:43 AM, Ina Panova  wrote:

> uuid sounds like good compromise.
>
>
>
> 
> Regards,
>
> Ina Panova
> Software Engineer| Pulp| Red Hat Inc.
>
> "Do not go where the path may lead,
>  go instead where there is no path and leave a trail."
>
> On Thu, Jun 14, 2018 at 9:38 PM, Jeff Ortel  wrote:
>
>>
>>
>> On 06/14/2018 12:19 PM, Jeff Ortel wrote:
>>
>>>
>>>
>>> On 06/14/2018 10:37 AM, Daniel Alley wrote:
>>>
 I will make one more suggestion.  What about naming "id" -> "uuid"?
 This carries the clear connotation that it is a unique identifier so it is
 less likely to be confusing a la "id and _id", and is still less likely to
 have a namespace conflict.

>>>
>>> Appreciate the suggestion but this would only be marginally less
>>> confusing.
>>>
>>
>> Reconsidering this suggestion for the reasons you outlined.
>>
>>
>> ___
>> 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

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

[Daniel Alley]

I'm -1 on going the underscore idea, partly because of the aforementioned
confusion issue, but also partly because but I've noticed that in our API,
the "underscore" basically has a semantic meeting of "href, [which is]
generated on the fly, not stored in the db".

Specifically:

   - '_href'
   - '_added_href'
   - '_removed_href'
   - '_content_href'

So I think if we use a prefix, we should avoid using one that already has a
semantic meaning (I don't know whether we actually planned for that to be
the case, but I think it's a useful pattern / distinction and I don't think
we should mess with it).

On Mon, Jun 18, 2018 at 2:01 PM, Brian Bouterse  wrote:

> Having a user focus made me realize that it would be useful if a user
> could easily tell which attributes were common to all content units versus
> just that one content unit. When scripting for instance that is really
> useful to know. We could document the 5 attributes that platform provides,
> but when there are 20+ attributes on a subclassed content unit the
> underscores would provide an easy, consistent answer to this question. This
> is an additional reason separate from the the issue that our content
> attribute names are colliding (id at least for now). The underscore prefix
> would make collisions highly unlikely also. This problem is only scoped to
> the Content unit since that is the place where we expect a large number of
> subclassed attributes.
>
> For this reason I believe using the _ as the prefix will provide 2
> benefits. I wrote them here on this ticket:  https://pulp.plan.io/issues/
> 3704
>
> I am still +1 on adopting those changes for those reasons. More feedback
> is welcome given the additional problem statements and discussion.
>
> On Mon, Jun 18, 2018 at 8:43 AM, Ina Panova  wrote:
>
>> uuid sounds like good compromise.
>>
>>
>>
>> 
>> Regards,
>>
>> Ina Panova
>> Software Engineer| Pulp| Red Hat Inc.
>>
>> "Do not go where the path may lead,
>>  go instead where there is no path and leave a trail."
>>
>> On Thu, Jun 14, 2018 at 9:38 PM, Jeff Ortel  wrote:
>>
>>>
>>>
>>> On 06/14/2018 12:19 PM, Jeff Ortel wrote:
>>>


 On 06/14/2018 10:37 AM, Daniel Alley wrote:

> I will make one more suggestion.  What about naming "id" -> "uuid"?
> This carries the clear connotation that it is a unique identifier so it is
> less likely to be confusing a la "id and _id", and is still less likely to
> have a namespace conflict.
>

 Appreciate the suggestion but this would only be marginally less
 confusing.

>>>
>>> Reconsidering this suggestion for the reasons you outlined.
>>>
>>>
>>> ___
>>> 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
>
>
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Brian Bouterse]

That is a great reason to not use the underscore prefix also for that.
We've never formally called this out (that I know of) so that is useful.

A good prefix is tough to come up with. Part of the issue is that changing
MasterModel affects all models so a prefix like 'pulp_id' would affect all
models even though the collision concern is only for subclasses of Content.
A name like pulp_id would look very strange on a Distributor, Repository,
Remote, etc.

In terms of Content though, is anyone else concerned that 'created_at' and
'updated_at' will be very ambiguous to end users? Does it mean when the rpm
was made or when it was added to Pulp? I think this question will come up a
lot. I believe all this stems from us loading up Content with fields that
a) plugin writer's have to deal with and b) users have to figure out.

There is another option to consider. What if 'Content' didn't inherit the
fields from MasterModel and instead just defined pulp_id as the primary key
only for Cotnent? Content would still have the functionality of
MasterModel, just not the inherited fields. That would resolve the issues
without needing a convention or the need to 'separate' the fields. We (I
think) have to keep 'artifacts' and/or call them 'pulp_artifacts' because
Content units need a foreign key so we have to name it something. All code
could refer to the pk as 'pk', which is why Django has this feature to
handle differing pk field names. End users would clearly see the
pulp_artifacts and pulp_id on serialized Content units. Other pulp objects
would serialize with 'id', 'created_at', etc like we have today.

What do you all think about ^?

As an aside, calling the field 'pk' is not an option either. A little test
I ran throws this django error on the model definition: hello.my_model.pk:
(fields.E003) 'pk' is a reserved word that cannot be used as a field name.



On Mon, Jun 18, 2018 at 2:15 PM, Daniel Alley  wrote:

> I'm -1 on going the underscore idea, partly because of the aforementioned
> confusion issue, but also partly because but I've noticed that in our API,
> the "underscore" basically has a semantic meeting of "href, [which is]
> generated on the fly, not stored in the db".
>
> Specifically:
>
>- '_href'
>- '_added_href'
>- '_removed_href'
>- '_content_href'
>
> So I think if we use a prefix, we should avoid using one that already has
> a semantic meaning (I don't know whether we actually planned for that to be
> the case, but I think it's a useful pattern / distinction and I don't think
> we should mess with it).
>
> On Mon, Jun 18, 2018 at 2:01 PM, Brian Bouterse 
> wrote:
>
>> Having a user focus made me realize that it would be useful if a user
>> could easily tell which attributes were common to all content units versus
>> just that one content unit. When scripting for instance that is really
>> useful to know. We could document the 5 attributes that platform provides,
>> but when there are 20+ attributes on a subclassed content unit the
>> underscores would provide an easy, consistent answer to this question. This
>> is an additional reason separate from the the issue that our content
>> attribute names are colliding (id at least for now). The underscore prefix
>> would make collisions highly unlikely also. This problem is only scoped to
>> the Content unit since that is the place where we expect a large number of
>> subclassed attributes.
>>
>> For this reason I believe using the _ as the prefix will provide 2
>> benefits. I wrote them here on this ticket:
>> https://pulp.plan.io/issues/3704
>>
>> I am still +1 on adopting those changes for those reasons. More feedback
>> is welcome given the additional problem statements and discussion.
>>
>> On Mon, Jun 18, 2018 at 8:43 AM, Ina Panova  wrote:
>>
>>> uuid sounds like good compromise.
>>>
>>>
>>>
>>> 
>>> Regards,
>>>
>>> Ina Panova
>>> Software Engineer| Pulp| Red Hat Inc.
>>>
>>> "Do not go where the path may lead,
>>>  go instead where there is no path and leave a trail."
>>>
>>> On Thu, Jun 14, 2018 at 9:38 PM, Jeff Ortel  wrote:
>>>


 On 06/14/2018 12:19 PM, Jeff Ortel wrote:

>
>
> On 06/14/2018 10:37 AM, Daniel Alley wrote:
>
>> I will make one more suggestion.  What about naming "id" -> "uuid"?
>> This carries the clear connotation that it is a unique identifier so it 
>> is
>> less likely to be confusing a la "id and _id", and is still less likely 
>> to
>> have a namespace conflict.
>>
>
> Appreciate the suggestion but this would only be marginally less
> confusing.
>

 Reconsidering this suggestion for the reasons you outlined.


 ___
 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/mailm

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

[Jeremy Audet]

> I'm -1 on going the underscore idea, partly because of the aforementioned
> confusion issue, but also partly because but I've noticed that in our API,
> the "underscore" basically has a semantic meeting of "href, [which is]
> generated on the fly, not stored in the db".
>
> Specifically:
>
>- '_href'
>- '_added_href'
>- '_removed_href'
>- '_content_href'
>
> So I think if we use a prefix, we should avoid using one that already has
> a semantic meaning (I don't know whether we actually planned for that to be
> the case, but I think it's a useful pattern / distinction and I don't think
> we should mess with it).
>

Outside perspective: My experience with Python, JavaScript, Ruby, C++, and
so on has led me to believe that the leading underscore means "only touch
if you know what you're doing." However, the _href attribute is something
that I, as an end user, have to use all the time. Thus, the lesson I've
taken away from this naming convention is "pulp abuses naming conventions."
I certainly didn't think that the leading underscore means "generated on
the fly" or "some kind of href." Others might think similarly.
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Daniel Alley]

Another way of thinking of it would be: "don't store store this unless you
absolutely know that the base of the URL will never, ever change".  Storing
IDs is fine, storing hrefs may potentially not be, because it can change
out from underneath you.  I think it's actually a similar concept.

On Thu, Jun 21, 2018 at 9:58 AM, Jeremy Audet  wrote:

>
> I'm -1 on going the underscore idea, partly because of the aforementioned
>> confusion issue, but also partly because but I've noticed that in our API,
>> the "underscore" basically has a semantic meeting of "href, [which is]
>> generated on the fly, not stored in the db".
>>
>> Specifically:
>>
>>- '_href'
>>- '_added_href'
>>- '_removed_href'
>>- '_content_href'
>>
>> So I think if we use a prefix, we should avoid using one that already has
>> a semantic meaning (I don't know whether we actually planned for that to be
>> the case, but I think it's a useful pattern / distinction and I don't think
>> we should mess with it).
>>
>
> Outside perspective: My experience with Python, JavaScript, Ruby, C++, and
> so on has led me to believe that the leading underscore means "only touch
> if you know what you're doing." However, the _href attribute is something
> that I, as an end user, have to use all the time. Thus, the lesson I've
> taken away from this naming convention is "pulp abuses naming conventions."
> I certainly didn't think that the leading underscore means "generated on
> the fly" or "some kind of href." Others might think similarly.
>
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Jeremy Audet]

Base URLs should never change. That's an expectation that all web
application clients everywhere should be able to rely on. "Cool URIs don't
change." If anything, storing IDs is the worse practice, because that
implies that the client is going to use pre-existing knowledge to locally
build URLs, instead of asking Pulp for the URLs it needs.
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Daniel Alley]

>
> Base URLs should never change. That's an expectation that all web
> application clients everywhere should be able to rely on.


Isn't changing the hostname something that downstream explicitly supports?
(I could be wrong here, I'm only recollecting random chats from months ago).

On Thu, Jun 21, 2018 at 11:26 AM, Jeremy Audet  wrote:

> Base URLs should never change. That's an expectation that all web
> application clients everywhere should be able to rely on. "Cool URIs don't
> change." If anything, storing IDs is the worse practice, because that
> implies that the client is going to use pre-existing knowledge to locally
> build URLs, instead of asking Pulp for the URLs it needs.
>
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[David Davis]

That’s correct. Katello has made it quite clear we have to support users
changing hostnames/port/etc.

David


On Fri, Jun 29, 2018 at 11:21 AM Daniel Alley  wrote:

> Base URLs should never change. That's an expectation that all web
>> application clients everywhere should be able to rely on.
>
>
> Isn't changing the hostname something that downstream explicitly
> supports?  (I could be wrong here, I'm only recollecting random chats from
> months ago).
>
> On Thu, Jun 21, 2018 at 11:26 AM, Jeremy Audet  wrote:
>
>> Base URLs should never change. That's an expectation that all web
>> application clients everywhere should be able to rely on. "Cool URIs don't
>> change." If anything, storing IDs is the worse practice, because that
>> implies that the client is going to use pre-existing knowledge to locally
>> build URLs, instead of asking Pulp for the URLs it needs.
>>
>
> ___
> 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

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

[Patrick Creech]

On Thu, 2018-06-21 at 11:26 -0400, Jeremy Audet wrote:
> Base URLs should never change. That's an expectation that all web application 
> clients everywhere should be able to rely on. "Cool URIs don't change." If 
> anything, storing IDs is the worse practice,
> because that implies that the client is going to use pre-existing knowledge 
> to locally build URLs, instead of asking Pulp for the URLs it needs.
> 
Just a minor note here.  While this is _definitely_ a great ideal for any 
public resource and is somtething that anything on the public web should try to 
obtain, this sentiment doesn't really hold
true to resources deployed internally at a company, in theory or in practice.  
Internal infrastructre at most companies changes, a lot.

Software deployed at a resource location should allow the flexibility for the 
owner to deploy it anywhere, move it anywhere, and do whatever it wants with 
it.  It should not lock down someones ability
to do this, or enforce this policy upon it's users deep in it's code/database.

I recently did this with a gitlab move I was involved in, renaming it from a 
location dependent domain name to a service style domain, and it most 
definitely allowed me to simply rename what hostname
it was deployed with via a single config variable, and still works on both 
hostnames at the same time.  I can also speak for the Atlassian suite of 
products, which also allow you to do this.  'Where'
something is deployed is up to the deployer, anything after that can and should 
be up to pulp.

signature.asc
Description: This is a digitally signed message part
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Jeff Ortel]

After long consideration, I have had a change of heart about this. I 
think.  In short, Pulp's data model has unique requirements that make it 
acceptable to deviate from common convention regarding ID as the PK.  
Mainly that the schema is extensible by plugin writers. Given the plugin 
architecture, I think it's reasonable to think of "core" fields like: 
ID, CREATED and LAST_MODIFIED as metadata. Although, the ID is harder to 
fit in this semantic, I think it's reasonable to do for consistency and 
to support the user query use-case re: content having an natural ID 
attribute.  Taking this further, the /href/ attributes /could/ be though 
of in the same category.


With this in mind, I'm thinking that the leading underscore (_) could be 
used broadly to denote /generated/ /or metadata/ fields and the 
following would be reasonable:


_id
_created
_last_updated

_href
_added_href
_removed_href
_content_href

I highly value consistency so this applies to the entire schema.

This will introduce a few fairly odd things into the schema that I 
/suppose/ we can live with.


- Two fields on /some/ tables named (ID ,  _ID).  To mitigate confusion, 
we should serialize the *pk* and not*_id*. This will also be consistent 
with *pk* parameters passed in.
- I expect django will generate foreign key fields with double 
understores.  Eg: content__id


I'm still -1 for using a /pulp_/ prefix.

Thoughts?


On 06/18/2018 01:15 PM, Daniel Alley wrote:
I'm -1 on going the underscore idea, partly because of the 
aforementioned confusion issue, but also partly because but I've 
noticed that in our API, the "underscore" basically has a semantic 
meeting of "href, [which is] generated on the fly, not stored in the db".


Specifically:

  * '_href'
  * '_added_href'
  * '_removed_href'
  * '_content_href'

So I think if we use a prefix, we should avoid using one that already 
has a semantic meaning (I don't know whether we actually planned for 
that to be the case, but I think it's a useful pattern / distinction 
and I don't think we should mess with it).


___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Daniel Alley]

>
> With this in mind, I'm thinking that the leading underscore (_) could be
> used broadly to denote *generated* *or metadata* fields and the following
> would be reasonable:
>

I'm on board with this, I think your revised description of what '_' could
symbolize makes a lot of sense.



On Tue, Aug 7, 2018 at 12:47 PM, Jeff Ortel  wrote:

> After long consideration, I have had a change of heart about this.  I
> think.  In short, Pulp's data model has unique requirements that make it
> acceptable to deviate from common convention regarding ID as the PK.
> Mainly that the schema is extensible by plugin writers.  Given the plugin
> architecture, I think it's reasonable to think of "core" fields like: ID,
> CREATED and LAST_MODIFIED as metadata.  Although, the ID is harder to fit
> in this semantic, I think it's reasonable to do for consistency and to
> support the user query use-case re: content having an natural ID
> attribute.  Taking this further, the *href* attributes *could* be though
> of in the same category.
>
> With this in mind, I'm thinking that the leading underscore (_) could be
> used broadly to denote *generated* *or metadata* fields and the following
> would be reasonable:
>
> _id
> _created
> _last_updated
>
> _href
> _added_href
> _removed_href
> _content_href
>
> I highly value consistency so this applies to the entire schema.
>
> This will introduce a few fairly odd things into the schema that I
> *suppose* we can live with.
>
> - Two fields on *some* tables named (ID ,  _ID).  To mitigate confusion,
> we should serialize the *pk* and not* _id*.  This will also be consistent
> with *pk* parameters passed in.
> - I expect django will generate foreign key fields with double
> understores.  Eg: content__id
>
> I'm still -1 for using a *pulp_* prefix.
>
> Thoughts?
>
>
> On 06/18/2018 01:15 PM, Daniel Alley wrote:
>
> I'm -1 on going the underscore idea, partly because of the aforementioned
> confusion issue, but also partly because but I've noticed that in our API,
> the "underscore" basically has a semantic meeting of "href, [which is]
> generated on the fly, not stored in the db".
>
> Specifically:
>
>- '_href'
>- '_added_href'
>- '_removed_href'
>- '_content_href'
>
> So I think if we use a prefix, we should avoid using one that already has
> a semantic meaning (I don't know whether we actually planned for that to be
> the case, but I think it's a useful pattern / distinction and I don't think
> we should mess with it).
>
>
>
> ___
> 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

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

[Dana Walker]

While I have not given your suggestion a lot of thought yet, it seems
sound.  I'd been wondering about the state of this discussion recently and
had already decided I'm onboard with whatever the majority decides merely
to get us moving again.

I want us to make good decisions so as to increase adoption of Pulp, but
for that to even matter, we need it finished and to swap to Pulp 3 from
Pulp 2.  At this point, I'm willing to say let's just pick what most prefer
and move forward (and that goes for any of these design discussions) and if
given the power of hindsight we'd like to make different decisions, we can
do so in Pulp 4.

--Dana

Dana Walker

Associate Software Engineer

Red Hat




On Tue, Aug 7, 2018 at 1:23 PM, Daniel Alley  wrote:

> With this in mind, I'm thinking that the leading underscore (_) could be
>> used broadly to denote *generated* *or metadata* fields and the
>> following would be reasonable:
>>
>
> I'm on board with this, I think your revised description of what '_' could
> symbolize makes a lot of sense.
>
>
>
> On Tue, Aug 7, 2018 at 12:47 PM, Jeff Ortel  wrote:
>
>> After long consideration, I have had a change of heart about this.  I
>> think.  In short, Pulp's data model has unique requirements that make it
>> acceptable to deviate from common convention regarding ID as the PK.
>> Mainly that the schema is extensible by plugin writers.  Given the plugin
>> architecture, I think it's reasonable to think of "core" fields like: ID,
>> CREATED and LAST_MODIFIED as metadata.  Although, the ID is harder to fit
>> in this semantic, I think it's reasonable to do for consistency and to
>> support the user query use-case re: content having an natural ID
>> attribute.  Taking this further, the *href* attributes *could* be though
>> of in the same category.
>>
>> With this in mind, I'm thinking that the leading underscore (_) could be
>> used broadly to denote *generated* *or metadata* fields and the
>> following would be reasonable:
>>
>> _id
>> _created
>> _last_updated
>>
>> _href
>> _added_href
>> _removed_href
>> _content_href
>>
>> I highly value consistency so this applies to the entire schema.
>>
>> This will introduce a few fairly odd things into the schema that I
>> *suppose* we can live with.
>>
>> - Two fields on *some* tables named (ID ,  _ID).  To mitigate confusion,
>> we should serialize the *pk* and not* _id*.  This will also be
>> consistent with *pk* parameters passed in.
>> - I expect django will generate foreign key fields with double
>> understores.  Eg: content__id
>>
>> I'm still -1 for using a *pulp_* prefix.
>>
>> Thoughts?
>>
>>
>> On 06/18/2018 01:15 PM, Daniel Alley wrote:
>>
>> I'm -1 on going the underscore idea, partly because of the aforementioned
>> confusion issue, but also partly because but I've noticed that in our API,
>> the "underscore" basically has a semantic meeting of "href, [which is]
>> generated on the fly, not stored in the db".
>>
>> Specifically:
>>
>>- '_href'
>>- '_added_href'
>>- '_removed_href'
>>- '_content_href'
>>
>> So I think if we use a prefix, we should avoid using one that already has
>> a semantic meaning (I don't know whether we actually planned for that to be
>> the case, but I think it's a useful pattern / distinction and I don't think
>> we should mess with it).
>>
>>
>>
>> ___
>> 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

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

[Milan Kovacik]

On Tue, Aug 7, 2018 at 6:47 PM, Jeff Ortel  wrote:

> After long consideration, I have had a change of heart about this.  I
> think.  In short, Pulp's data model has unique requirements that make it
> acceptable to deviate from common convention regarding ID as the PK.
> Mainly that the schema is extensible by plugin writers.  Given the plugin
> architecture, I think it's reasonable to think of "core" fields like: ID,
> CREATED and LAST_MODIFIED as metadata.  Although, the ID is harder to fit
> in this semantic, I think it's reasonable to do for consistency and to
> support the user query use-case re: content having an natural ID
> attribute.  Taking this further, the *href* attributes *could* be though
> of in the same category.
>
> With this in mind, I'm thinking that the leading underscore (_) could be
> used broadly to denote *generated* *or metadata* fields and the following
> would be reasonable:
>
> _id
> _created
> _last_updated
>
> _href
> _added_href
> _removed_href
> _content_href
>
> I highly value consistency so this applies to the entire schema.
>

I'd even go as far as making a distinct core attribute of a plug-in
provided model: rpm.core.id, rpm.core.href, namespacing the model and
promoting composition.
This would have the benefit of being able to easily distinguish between
platform and plug-in specific data especially if plug-in specific data is
expected to be the main user interaction site i.e user_importance(rpm.id) >
user_importance(rpm.core.id).

 --
milan


> This will introduce a few fairly odd things into the schema that I
> *suppose* we can live with.
>
> - Two fields on *some* tables named (ID ,  _ID).  To mitigate confusion,
> we should serialize the *pk* and not* _id*.  This will also be consistent
> with *pk* parameters passed in.
> - I expect django will generate foreign key fields with double
> understores.  Eg: content__id
>

> I'm still -1 for using a *pulp_* prefix.
>
> Thoughts?
>
>
> On 06/18/2018 01:15 PM, Daniel Alley wrote:
>
> I'm -1 on going the underscore idea, partly because of the aforementioned
> confusion issue, but also partly because but I've noticed that in our API,
> the "underscore" basically has a semantic meeting of "href, [which is]
> generated on the fly, not stored in the db".
>
> Specifically:
>
>- '_href'
>- '_added_href'
>- '_removed_href'
>- '_content_href'
>
> So I think if we use a prefix, we should avoid using one that already has
> a semantic meaning (I don't know whether we actually planned for that to be
> the case, but I think it's a useful pattern / distinction and I don't think
> we should mess with it).
>
>
>
> ___
> 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

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

[Jeff Ortel]

On 08/07/2018 11:47 AM, Jeff Ortel wrote:
After long consideration, I have had a change of heart about this.  I 
think.  In short, Pulp's data model has unique requirements that make 
it acceptable to deviate from common convention regarding ID as the 
PK.  Mainly that the schema is extensible by plugin writers.  Given 
the plugin architecture, I think it's reasonable to think of "core" 
fields like: ID, CREATED and LAST_MODIFIED as metadata.  Although, the 
ID is harder to fit in this semantic, I think it's reasonable to do 
for consistency and to support the user query use-case re: content 
having an natural ID attribute. Taking this further, the /href/ 
attributes /could/ be though of in the same category.


With this in mind, I'm thinking that the leading underscore (_) could 
be used broadly to denote /generated/ /or metadata/ fields and the 
following would be reasonable:


_id
_created
_last_updated


I'm convinced that all tables should have _created.  Knowing when 
something is created helps fulfill many common use cases and is 
essential for troubleshooting.  I am open to including _last_updated 
only on mutable entities .  Depending on the number (ratio) of 
mutable/immutable entities, we could support this with either an 
additional Model class eg: MutableModel or just add _last_updated on 
concrete models.  Either way, the column (attribute) needs to be named 
consistently.




_href
_added_href
_removed_href
_content_href

I highly value consistency so this applies to the entire schema.

This will introduce a few fairly odd things into the schema that I 
/suppose/ we can live with.


- Two fields on /some/ tables named (ID ,  _ID).  To mitigate 
confusion, we should serialize the *pk* and not*_id*.  This will also 
be consistent with *pk* parameters passed in.
- I expect django will generate foreign key fields with double 
understores.  Eg: content__id


I'm still -1 for using a /pulp_/ prefix.

Thoughts?


___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

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

[Brian Bouterse]

I'm currently reworking the Content unit to not use MasterDetail (
https://pulp.plan.io/issues/3812). I will make the changes I think I heard
on this thread as part of that work. Specifically I plan to have the top
level Pulp model that all objects inherit from define:  _id, _created, and
_updated. Let me know if I should not do this.

re creating mutable vs immutable model types: I don't think it's worth the
complexity.

On Mon, Aug 13, 2018 at 10:38 AM, Jeff Ortel  wrote:

>
>
> On 08/07/2018 11:47 AM, Jeff Ortel wrote:
>
> After long consideration, I have had a change of heart about this.  I
> think.  In short, Pulp's data model has unique requirements that make it
> acceptable to deviate from common convention regarding ID as the PK.
> Mainly that the schema is extensible by plugin writers.  Given the plugin
> architecture, I think it's reasonable to think of "core" fields like: ID,
> CREATED and LAST_MODIFIED as metadata.  Although, the ID is harder to fit
> in this semantic, I think it's reasonable to do for consistency and to
> support the user query use-case re: content having an natural ID
> attribute.  Taking this further, the *href* attributes *could* be though
> of in the same category.
>
> With this in mind, I'm thinking that the leading underscore (_) could be
> used broadly to denote *generated* *or metadata* fields and the following
> would be reasonable:
>
> _id
> _created
> _last_updated
>
>
> I'm convinced that all tables should have _created.  Knowing when
> something is created helps fulfill many common use cases and is essential
> for troubleshooting.  I am open to including _last_updated only on mutable
> entities .  Depending on the number (ratio) of mutable/immutable entities,
> we could support this with either an additional Model class eg:
> MutableModel or just add _last_updated on concrete models.  Either way, the
> column (attribute) needs to be named consistently.
>
>
> _href
> _added_href
> _removed_href
> _content_href
>
> I highly value consistency so this applies to the entire schema.
>
> This will introduce a few fairly odd things into the schema that I
> *suppose* we can live with.
>
> - Two fields on *some* tables named (ID ,  _ID).  To mitigate confusion,
> we should serialize the *pk* and not* _id*.  This will also be consistent
> with *pk* parameters passed in.
> - I expect django will generate foreign key fields with double
> understores.  Eg: content__id
>
> I'm still -1 for using a *pulp_* prefix.
>
> Thoughts?
>
>
>
> ___
> 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