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

Brian Bouterse bbouters at redhat.com
Mon Jun 18 19:30:54 UTC 2018


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 <dalley at redhat.com> 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 <bbouters at redhat.com>
> 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 <ipanova at redhat.com> 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 <jortel at redhat.com> 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 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/20180618/0b0ffee9/attachment.htm>


More information about the Pulp-dev mailing list