It would be automatic, and plugins need a change only to avoid redundant prepending. E.g. If RPM plugin makes no changes, the endpoint for RPM content will be:
/api/v3/content/pulp_rpm/rpm/packages/ because endpoint_name = 'rpm/packages'. So plugin should leave only endpoint_name = 'packages'. The endpoint with redundant plugin name will work fine, just doesn't look good :) Tanya On Wed, Jan 2, 2019 at 7:20 PM David Davis <davidda...@redhat.com> wrote: > I am +1 to namespacing all master detail models with the plugin name. > Would this be automatic or something that the plugin writers would be > encouraged to do? > > David > > > On Wed, Jan 2, 2019 at 12:58 PM Tatiana Tereshchenko <ttere...@redhat.com> > wrote: > >> Thank you all for the discussion so far. >> The question - the type field and namespacing in content summary - is >> solved with https://pulp.plan.io/issues/4185. >> >> The last remaining question is whether we want to prepend endpoints for >> master/detail models with plugin label. If yes, then everything or for >> Content only. >> See details on the issue https://pulp.plan.io/issues/4279. >> >> Examples of the suggested change: >> >> /api/v3/content/rpm/packages/ --> /api/v3/content/pulp_rpm/packages/ >> /api/v3/remotes/rpm/ --> /api/v3/content/remotes/pulp_rpm/rpm/ >> /api/v3/publishers/rpm/ --> /api/v3/content/publishers/pulp_rpm/rpm/ >> >> Changes which will be needed in plugins: >> - adjust the value of the `endpoint_name` attribute in the viewsets we >> introduce changes to >> >> Please provide feedback, here or on the issue >> https://pulp.plan.io/issues/4279. >> This is an RC blocker, so it would be great to groom it over the next >> couple of days. >> >> Thank you, >> Tanya >> >> On Thu, Dec 20, 2018 at 9:41 AM Tatiana Tereshchenko <ttere...@redhat.com> >> wrote: >> >>> Since we are leaning towards prepending types for _all_ master/detail >>> models and not only for the content model, that Django fix is no longer >>> important for us. >>> >>> Tanya >>> >>> On Wed, Dec 19, 2018 at 6:18 PM Daniel Alley <dal...@redhat.com> wrote: >>> >>>> David, was that a vote to make it explicit? >>>> >>>> I would regard this as fairly intuitive as far as "magic-ness" goes, >>>> acceptable from the user POV in my opinion. And if Django is explicitly >>>> trying to support this functionality and relies on it working properly, and >>>> has a unittest for it going forwards, then I'm fairly confident it won't be >>>> too fragile. My vote is +1. >>>> >>>> My only concern (and it's not a major one) is that a plugin that needed >>>> to be renamed might have problems with this. But I think that would be >>>> resolvable with a migration. >>>> >>>> Tanya, will we need to remove the workaround once Django 2.2 comes >>>> out? If so, we should file a Refactor task for it. >>>> >>>> On Tue, Dec 18, 2018 at 9:39 AM David Davis <davidda...@redhat.com> >>>> wrote: >>>> >>>>> +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 >>>>> >>>> _______________________________________________ >>>> 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
