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

2018-08-16 Thread 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

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

2018-08-13 Thread 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

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

2018-08-08 Thread 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

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

2018-08-07 Thread 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

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

2018-08-07 Thread 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,

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

2018-08-07 Thread 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

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

2018-06-29 Thread 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

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

2018-06-29 Thread 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. > >

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

2018-06-29 Thread 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

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

2018-06-21 Thread 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,

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

2018-06-21 Thread 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

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

2018-06-21 Thread 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: > >

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

2018-06-18 Thread 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

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

2018-06-18 Thread 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' -

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

2018-06-18 Thread 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

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

2018-06-18 Thread 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:

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

2018-06-15 Thread 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

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

2018-06-14 Thread 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

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

2018-06-14 Thread 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

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

2018-06-14 Thread 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

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

2018-06-14 Thread 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 -

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

2018-06-14 Thread 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

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

2018-06-14 Thread 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

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

2018-06-14 Thread 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

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

2018-06-13 Thread 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

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

2018-06-12 Thread 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

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

2018-06-12 Thread 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

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

2018-06-12 Thread 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

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

2018-06-12 Thread 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

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

2018-06-12 Thread 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: > >> >>

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

2018-06-12 Thread 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'

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

2018-06-11 Thread 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

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

2018-06-11 Thread 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.

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

2018-06-08 Thread 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

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

2018-06-08 Thread 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

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

2018-06-08 Thread 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

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

2018-06-08 Thread 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

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

2018-06-07 Thread 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

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

2018-05-29 Thread 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

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

2018-05-29 Thread 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

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

2018-05-25 Thread 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

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

2018-05-25 Thread 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

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

2018-05-24 Thread 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

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

2018-05-23 Thread 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

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

2018-05-23 Thread 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

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

2018-05-23 Thread 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

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

2018-05-23 Thread 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

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

2018-05-23 Thread 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