[Pulp-dev] pulp 3 bindings change proposal
Justin Sherrill
jsherril at redhat.com
Thu Jun 20 13:53:25 UTC 2019
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
> <mailto:dkliban at redhat.com>> wrote:
>
> On Wed, Jun 19, 2019 at 11:34 AM Dennis Kliban <dkliban at redhat.com
> <mailto:dkliban at redhat.com>> wrote:
>
> On Wed, Jun 19, 2019 at 11:20 AM Justin Sherrill
> <jsherril at redhat.com <mailto: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?
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 <mailto: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 list
>> Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>
>
> On Wed, Jun 19, 2019 at 1:57 PM Dennis Kliban <dkliban at redhat.com
> <mailto:dkliban at redhat.com>> wrote:
>
> On Wed, Jun 19, 2019 at 11:34 AM Dennis Kliban <dkliban at redhat.com
> <mailto:dkliban at redhat.com>> wrote:
>
> On Wed, Jun 19, 2019 at 11:20 AM Justin Sherrill
> <jsherril at redhat.com <mailto: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 <mailto: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 list
>> Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20190620/b9627058/attachment.htm>
More information about the Pulp-dev
mailing list