[Pulp-dev] Namespacing plugins, looking for feedback

Tue Dec 18 14:07:40 UTC 2018

Brian,
the current PR [0] does exactly what you describe, it uses label which is
taken from the plugin subclass of PulpPluginAppconfig + TYPE defined on the
detail model.
FWIW, there is an option to use plugin class name and not a plugin writer
defined TYPE, e.g. pulp_file.filecontent, pulp_rpm.package,
pulp_rpm.updaterecord, etc.

Jeff, to answer your questions:

> 1. why do all the plugins begin with "pulp_"?
>
This is how django app label is defined in every plugin so far, see
pulp_file case [1].
Whatever is defined there is used as a plugin name.

2. can the plugin name get pre-pended when it's loaded by core?
>
> I lean toward TYPE=<plugin>.<type>

Just to clarify, there is a class arttriburte `TYPE` and there is a `type`
field on a model. I guess you suggest type = <plugin>.<TYPE>.

We can probably do it on a master model in the save method [2], just
initially the change was proposed for Content models only.
If we decide to namespace all master/detail objects, I agree we can do it n
a more generic way, than just redefine __init__ on a specific class.

Tanya

[0]  https://github.com/pulp/pulp/pull/3801
[1]
https://github.com/pulp/pulp_file/blob/24881314372b9c1c505ff687c15238126b261afa/pulp_file/app/__init__.py#L10
[2]
https://github.com/pulp/pulp/blob/master/pulpcore/app/models/base.py#L76-L83

On Tue, Dec 18, 2018 at 12:58 PM Ina Panova <ipanova at redhat.com> wrote:

> +1 to namespace master/detail as well.
> +1 to Brian's suggestion to try.
>
> --------
> 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 Tue, Dec 18, 2018 at 12:15 AM Brian Bouterse <bbouters at redhat.com>
> wrote:
>
>> +1 to namespacing all Master/Detail objects (Remotes, Publishers, etc).
>> Namespacing will increase consistency w/ the user experience and will avoid
>> plugin-to-plugin naming collisions. @ttereshc +1 to the url changes and
>> content summary changes you've described.
>>
>> I think it would be ideal if the app specified its 'label' attribute on
>> the PulpPluginAppconfig subclass, e.g here in pulp_file
>> https://github.com/pulp/pulp_file/blob/24881314372b9c1c505ff687c15238126b261afa/pulp_file/app/__init__.py#L10
>> Then the Model for, e.g. the FileContent would have the second portion of
>> the string 'file' as an example and Master/Detail would assemble them.
>>
>> Is this implementation how you imagined it?
>>
>>
>>
>>
>> On Mon, Dec 17, 2018 at 9:29 AM Tatiana Tereshchenko <ttereshc at redhat.com>
>> wrote:
>>
>>> Just to clarify, the type field is not used in the endpoint
>>> construction, so two changes described in the original e-mail are
>>> independent.
>>>
>>> In my opinion:
>>>  - it is possible to have type collisions.
>>>  - it is possible to have the same endpoints (endpoint_name in a
>>> viewset).
>>>
>>> FWIW, the endpoint collision is not unique to the master/detail models'
>>> endpoints. A plugin, in theory, can define any endpoint they want.
>>> Though not preventing collisions it for endpoints related to
>>> master/detail models makes it easier to create such collision accidentally.
>>>
>>> Tanya
>>>
>>> On Mon, Dec 17, 2018 at 2:27 PM David Davis <daviddavis at redhat.com>
>>> wrote:
>>>
>>>> Is it possible (under the current model, without namespacing) to have
>>>> type collisions in the database for master/detail models? Like what if two
>>>> plugins define two Contents with the same type or two Remotes with the same
>>>> type? This kind of leads me to believe we should namespace everything. On
>>>> the Ansible plugin for example, I started working on a git Remote[0].
>>>> Luckily I chose "ansible_git" as the type but I could see plugin writers
>>>> running into problems if they are not so careful.
>>>>
>>>> [0]
>>>> https://github.com/pulp/pulp_ansible/pull/38/files#diff-debb42c875c19140793de39be3696ee3
>>>>
>>>> David
>>>>
>>>>
>>>> On Sun, Dec 16, 2018 at 4:41 PM Tatiana Tereshchenko <
>>>> ttereshc at redhat.com> wrote:
>>>>
>>>>> There is an issue [0] of colliding type names in the content summary
>>>>> which evolved into more general namespacing problem for plugins.
>>>>>
>>>>> The suggested changes [1] are:
>>>>>  1. include plugin name into the content summary
>>>>>
>>>>> "content_summary": {
>>>>>     "pulp_rpm.package": 50,
>>>>>     "pulp_rpm.errata": 2,
>>>>>     "pulp_file.file": 5
>>>>> }
>>>>>
>>>>>
>>>>> 2. include plugin name into content endpoints
>>>>> /api/v3/content/file/files/ --> /api/v3/content/pulp_file/files/
>>>>> /api/v3/content/rpm/packages/ --> /api/v3/content/pulp_rpm/packages/
>>>>> /api/v3/content/rpm/errata/ --> /api/v3/content/pulp_rpm/errata/
>>>>> ...
>>>>>
>>>>> For the change #1, not only content summary output is changed but the
>>>>> type itself in the database. If the content type is used somewhere in the
>>>>> filters, it should be specified in that format: "plugin_name.plugin_type".
>>>>> Does it makes sense to extend the master model and have a plugin name field
>>>>> and a type field, instead of putting preformatted string into the type
>>>>> field?
>>>>>
>>>>> For the change #2, endpoints are namespaced only for the content
>>>>> endpoint and not for other endpoints related to master/detail models, like
>>>>> remotes, publishers, etc. It's inconsistent, however it makes the most
>>>>> sense to have it for content endpoints.
>>>>>
>>>>> Any concerns or thoughts?
>>>>>
>>>>> Thank you,
>>>>> Tanya
>>>>>
>>>>> [0] https://pulp.plan.io/issues/4185#note-8
>>>>> [1] https://github.com/pulp/pulp/pull/3801
>>>>> _______________________________________________
>>>>> 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/20181218/8632baa9/attachment.htm>