+1 David
On Tue, Dec 18, 2018 at 9:31 AM Brian Bouterse <bbout...@redhat.com> wrote: > There is also an issue w/ my suggestion in that it's highly magical. The > class name is likely going to go through a case mutation and if not it's > going to be finicky in terms of its case. So now I'm thinking the plugin > writer should have to define it to keep it simple and explicit (not > implicit and magical). > > On Tue, Dec 18, 2018 at 9:27 AM David Davis <davidda...@redhat.com> wrote: > >> Would it be possible to default to class name but let plugin writers >> override this? I would imagine in some cases plugin writers might want to >> change the name (eg cases where you can't use type as the class name like >> File). >> >> David >> >> >> On Tue, Dec 18, 2018 at 9:23 AM Brian Bouterse <bbout...@redhat.com> >> wrote: >> >>> >>> >>> On Tue, Dec 18, 2018 at 9:07 AM Tatiana Tereshchenko < >>> ttere...@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 <ipan...@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 <bbout...@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 < >>>>>> ttere...@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 <davidda...@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 < >>>>>>>> ttere...@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@redhat.com >>>>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>>>>>> >>>>>>>> _______________________________________________ >>>>>>> Pulp-dev mailing list >>>>>>> Pulp-dev@redhat.com >>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>>>> >>>>>> _______________________________________________ >>>>>> Pulp-dev mailing list >>>>>> Pulp-dev@redhat.com >>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>>>> >>>>> _______________________________________________ >>> Pulp-dev mailing list >>> Pulp-dev@redhat.com >>> https://www.redhat.com/mailman/listinfo/pulp-dev >>> >>
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
