[Pulp-dev] Namespacing plugins, looking for feedback

Tue Dec 18 14:23:31 UTC 2018

On Tue, Dec 18, 2018 at 9:07 AM Tatiana Tereshchenko <ttereshc at redhat.com>
wrote:

> 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.
>
+1 to using the classname. Having the plugin class name used would allow
the user to not repeat themselves as much. I think it's likely the class
name == TYPE in almost all cases. The plugin writer would have 1 less
requirement on them at Content model definition time and that helps achieve
the "less burden on plugin writers" goal for pulp.

>
> 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/ae218a0a/attachment.htm>