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

>>>
>>>
>>> ___
>>> 

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

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

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

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

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