On Tue, Dec 18, 2018 at 9:07 AM Tatiana Tereshchenko <[email protected]> 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 <[email protected]> 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 <[email protected]> >> 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 < >>> [email protected]> 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 <[email protected]> >>>> 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 < >>>>> [email protected]> 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 >>>>>> [email protected] >>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>>> >>>>> _______________________________________________ >>>> Pulp-dev mailing list >>>> [email protected] >>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>> >>> _______________________________________________ >>> Pulp-dev mailing list >>> [email protected] >>> https://www.redhat.com/mailman/listinfo/pulp-dev >>> >>
_______________________________________________ Pulp-dev mailing list [email protected] https://www.redhat.com/mailman/listinfo/pulp-dev
