On Thu, Jun 20, 2019 at 9:53 AM Justin Sherrill <jsher...@redhat.com> wrote:
> > On 6/20/19 8:02 AM, Dennis Kliban wrote: > > On Wed, Jun 19, 2019 at 1:57 PM Dennis Kliban <dkli...@redhat.com> wrote: > >> On Wed, Jun 19, 2019 at 11:34 AM Dennis Kliban <dkli...@redhat.com> >> wrote: >> >>> On Wed, Jun 19, 2019 at 11:20 AM Justin Sherrill <jsher...@redhat.com> >>> wrote: >>> >>>> If a plugin provided multiple remotes, for example, what would that >>>> look like? >>>> >>>> in your example: >>>> >>>> -file_remote = >>>> fileremotes.remotes_file_file_create(remote_data)+file_remote = >>>> fileremotes.create(remote_data) >>>> >>>> Lets say the file plugin provided some other remote that still synced file >>>> content? >>>> >>>> >>> The goal is to provide separate API objects for each remote or content >>> type that a plugin provides. So the code would look like this: >>> >>> file_remote = fileremote.create(remote_data) >>> file_fancy_remote = filefancyremote.create(fancy_remote_data) >>> >>> My current implementation does not support this, but I am working toward >>> the above solution. >>> >> >> I was able to achieve this. I posted some screen shots of what the docs >> look like here[0]. >> >> Docker has multiple content types. So docker bindings would provide the >> following objects: ContentDockerBlobsApi, ContentDockerManifestListTagsApi, >> ContentDockerManifestListsApi, ContentDockerManifestTagsApi, and >> ContentDockerManifestsApi. >> > >> > I updated my patch and removed the plugin name from the Api object names. > So the above objects are now ContentBlobsApi, ContentManifestListTagsApi, > ContentManifestListsApi, ContentManifestTagsApi, and ContentManifestsApi. > > I like this all, and agree it improves readability. I assume there's no > concern about plugins implementing some model with the same name? Or i > guess this could already be a problem when it comes to model/db table names > in the app itself? > Each plugin ships its own client library. So even if the object names collide, they will be provided by different packages. > Justin > > > I have 2 PRs for this change[0,1]. The use of the bindings can be seen in > both of the PR. I'd like to get this work merged today. > > [0] https://github.com/pulp/pulpcore/pull/178 > [1] https://github.com/pulp/pulp-openapi-generator/pull/18 > > > Each of those objects would have a create(), read(), delete(), list() >> methods. >> >> Do others agree that this improves the usability of the bindings? >> >> >> [0] https://imgur.com/a/Ag7gqmj >> >> >>> >>> >>>> >>>> Justin >>>> >>>> On 6/19/19 9:45 AM, Dennis Kliban wrote: >>>> >>>> I didn't get a note in my email, but I did see one on in the list >>>> archive[0]. So here is my response to it: >>>> >>>> I agree that we could use modified templates to achieve the same >>>> results. However, that means that we will need to modify templates for >>>> every language we want to generate bindings in. In both cases the generated >>>> client code will be exactly the same. From a maintenance perspective, it is >>>> easier to add a feature to Pulp's REST API that produces a modified version >>>> of the OpenAPI schema. It also means that we can always use the latest >>>> versions of the templates shipped with openapi-generator. >>>> >>>> The documentation site would continue to distribute an OpenAPI schema >>>> where each Operation Id is unique. >>>> >>>> Pulp's OpenAPI schema does not currently pass validation because the >>>> paths are not unique. In order to use the 'href' of each resource as the >>>> primary identifier, it was necessary to template paths as {artifact_href}, >>>> {repository_href}, {file_content_href}, etc. This schema cannot be used to >>>> generate server code. However, it works well when generating client code. >>>> The non-unique operation ids would be a problem for generating a server >>>> also. However, they don't produce problems when generating client code. >>>> >>>> Does this address your concerns? >>>> >>>> [0] https://www.redhat.com/archives/pulp-dev/2019-June/msg00061.html >>>> >>>> On Wed, Jun 19, 2019 at 8:54 AM Dennis Kliban <dkli...@redhat.com> >>>> wrote: >>>> >>>>> As pointed out in a recent issue[0], the method names in the bindings >>>>> generated from Pulp's OpenAPI schema are unnecessarily verbose. Each >>>>> method >>>>> name corresponds to an Operation Id in the OpenAPI schema. The Operation >>>>> Id >>>>> is also used as an HTML anchor in the REST API docs[1]. >>>>> >>>>> It is possible to generate a schema where each Operation Id is >>>>> shorter, but then the Operation Ids are not unique and all the linking in >>>>> the REST API documentation breaks. We can avoid this problem by keeping >>>>> the >>>>> long Operation Id for the schema generated for the docs and only using >>>>> short Operation Ids when generating the schema for the bindings. >>>>> >>>>> The difference in usage of the bindings can be seen here[2]. >>>>> >>>>> Is there any objection to including such a change in time for RC 3? >>>>> >>>>> [0] https://pulp.plan.io/issues/4989 >>>>> [1] https://docs.pulpproject.org/en/3.0/nightly/restapi.html >>>>> [2] https://pulp.plan.io/issues/4989#note-1 >>>>> >>>> >>>> _______________________________________________ >>>> Pulp-dev mailing >>>> listPulp-dev@redhat.comhttps://www.redhat.com/mailman/listinfo/pulp-dev >>>> >>>> > On Wed, Jun 19, 2019 at 1:57 PM Dennis Kliban <dkli...@redhat.com> wrote: > >> On Wed, Jun 19, 2019 at 11:34 AM Dennis Kliban <dkli...@redhat.com> >> wrote: >> >>> On Wed, Jun 19, 2019 at 11:20 AM Justin Sherrill <jsher...@redhat.com> >>> wrote: >>> >>>> If a plugin provided multiple remotes, for example, what would that >>>> look like? >>>> >>>> in your example: >>>> >>>> -file_remote = >>>> fileremotes.remotes_file_file_create(remote_data)+file_remote = >>>> fileremotes.create(remote_data) >>>> >>>> Lets say the file plugin provided some other remote that still synced file >>>> content? >>>> >>>> >>> The goal is to provide separate API objects for each remote or content >>> type that a plugin provides. So the code would look like this: >>> >>> file_remote = fileremote.create(remote_data) >>> file_fancy_remote = filefancyremote.create(fancy_remote_data) >>> >>> My current implementation does not support this, but I am working toward >>> the above solution. >>> >> >> I was able to achieve this. I posted some screen shots of what the docs >> look like here[0]. >> >> Docker has multiple content types. So docker bindings would provide the >> following objects: ContentDockerBlobsApi, ContentDockerManifestListTagsApi, >> ContentDockerManifestListsApi, ContentDockerManifestTagsApi, and >> ContentDockerManifestsApi. >> >> Each of those objects would have a create(), read(), delete(), list() >> methods. >> >> Do others agree that this improves the usability of the bindings? >> >> >> [0] https://imgur.com/a/Ag7gqmj >> >> >>> >>> >>>> >>>> Justin >>>> >>>> On 6/19/19 9:45 AM, Dennis Kliban wrote: >>>> >>>> I didn't get a note in my email, but I did see one on in the list >>>> archive[0]. So here is my response to it: >>>> >>>> I agree that we could use modified templates to achieve the same >>>> results. However, that means that we will need to modify templates for >>>> every language we want to generate bindings in. In both cases the generated >>>> client code will be exactly the same. From a maintenance perspective, it is >>>> easier to add a feature to Pulp's REST API that produces a modified version >>>> of the OpenAPI schema. It also means that we can always use the latest >>>> versions of the templates shipped with openapi-generator. >>>> >>>> The documentation site would continue to distribute an OpenAPI schema >>>> where each Operation Id is unique. >>>> >>>> Pulp's OpenAPI schema does not currently pass validation because the >>>> paths are not unique. In order to use the 'href' of each resource as the >>>> primary identifier, it was necessary to template paths as {artifact_href}, >>>> {repository_href}, {file_content_href}, etc. This schema cannot be used to >>>> generate server code. However, it works well when generating client code. >>>> The non-unique operation ids would be a problem for generating a server >>>> also. However, they don't produce problems when generating client code. >>>> >>>> Does this address your concerns? >>>> >>>> [0] https://www.redhat.com/archives/pulp-dev/2019-June/msg00061.html >>>> >>>> On Wed, Jun 19, 2019 at 8:54 AM Dennis Kliban <dkli...@redhat.com> >>>> wrote: >>>> >>>>> As pointed out in a recent issue[0], the method names in the bindings >>>>> generated from Pulp's OpenAPI schema are unnecessarily verbose. Each >>>>> method >>>>> name corresponds to an Operation Id in the OpenAPI schema. The Operation >>>>> Id >>>>> is also used as an HTML anchor in the REST API docs[1]. >>>>> >>>>> It is possible to generate a schema where each Operation Id is >>>>> shorter, but then the Operation Ids are not unique and all the linking in >>>>> the REST API documentation breaks. We can avoid this problem by keeping >>>>> the >>>>> long Operation Id for the schema generated for the docs and only using >>>>> short Operation Ids when generating the schema for the bindings. >>>>> >>>>> The difference in usage of the bindings can be seen here[2]. >>>>> >>>>> Is there any objection to including such a change in time for RC 3? >>>>> >>>>> [0] https://pulp.plan.io/issues/4989 >>>>> [1] https://docs.pulpproject.org/en/3.0/nightly/restapi.html >>>>> [2] https://pulp.plan.io/issues/4989#note-1 >>>>> >>>> >>>> _______________________________________________ >>>> Pulp-dev mailing >>>> listPulp-dev@redhat.comhttps://www.redhat.com/mailman/listinfo/pulp-dev >>>> >>>>
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
