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

Thu Jun 14 14:55:45 UTC 2018

On Thu, Jun 14, 2018 at 10:28 AM, Dana Walker <dawalker at redhat.com> 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
>
> <https://www.redhat.com>
> <https://red.ht/sig>
>
> On Thu, Jun 14, 2018 at 9:08 AM, Brian Bouterse <bbouters at redhat.com>
> 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 <jortel at redhat.com> 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 <bbouters at redhat.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Jun 12, 2018 at 5:11 PM, David Davis <daviddavis at redhat.com>
>>>> wrote:
>>>>
>>>>> On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse <bbouters at 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 <jortel at 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
>>>> 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 at redhat.com
>>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Pulp-dev mailing list
>>>>>> Pulp-dev at redhat.com
>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>> _______________________________________________
>>> Pulp-dev mailing list
>>> Pulp-dev at redhat.com
>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>
>>>
>>
>> _______________________________________________
>> Pulp-dev mailing list
>> Pulp-dev at redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
>>
>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180614/9a91763b/attachment.htm>