[Pulp-dev] pulp 3 bindings change proposal

Thu Jun 20 14:12:25 UTC 2019

On Thu, Jun 20, 2019 at 9:53 AM Justin Sherrill <jsherril at redhat.com> wrote:

>
> On 6/20/19 8:02 AM, Dennis Kliban wrote:
>
> On Wed, Jun 19, 2019 at 1:57 PM Dennis Kliban <dkliban at redhat.com> wrote:
>
>> On Wed, Jun 19, 2019 at 11:34 AM Dennis Kliban <dkliban at redhat.com>
>> wrote:
>>
>>> On Wed, Jun 19, 2019 at 11:20 AM Justin Sherrill <jsherril at 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 <dkliban at 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 at redhat.comhttps://www.redhat.com/mailman/listinfo/pulp-dev
>>>>
>>>>
> On Wed, Jun 19, 2019 at 1:57 PM Dennis Kliban <dkliban at redhat.com> wrote:
>
>> On Wed, Jun 19, 2019 at 11:34 AM Dennis Kliban <dkliban at redhat.com>
>> wrote:
>>
>>> On Wed, Jun 19, 2019 at 11:20 AM Justin Sherrill <jsherril at 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 <dkliban at 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 at redhat.comhttps://www.redhat.com/mailman/listinfo/pulp-dev
>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20190620/09a7f7ec/attachment.htm>