Correct me if I’m wrong but Pulp 2 supported @bizhang’s model of providing both hrefs and ids. Was that a source of problems or complaints by Pulp 2 users?
David On Wed, Jul 25, 2018 at 3:08 PM Dennis Kliban <dkli...@redhat.com> wrote: > For everyone following along, the conversation has moved to Github - on > the PR[0] with the proposed changes. > > [0] https://github.com/pulp/pulp/pull/3561 > > On Tue, Jul 24, 2018 at 11:15 AM, Bihan Zhang <bizh...@redhat.com> wrote: > >> @dkliban I've tried out your PR and left a question: >> https://github.com/pulp/pulp/pull/3561#issuecomment-407425172 >> >> Won't it be problematic with the openapi definitions causing us to have >>> two schemas? Accepting the data in two forms is one thing, but using >>> openapi to describe it both ways is something I don't understand well. Are >>> we going to ship and test two? >>> >> >> I don't think we'll be defining the data in two different ways in >> openapi. We need to pass a {repository identifier} to /sync/, openapi >> expects a string, what we do with that string is up to us. (In the >> following example the format is "uri" but this isn't actually used for >> validation at all, since it's not defined by the swagger specification [0], >> we can also clear out the format field, since format is only there to >> support documentation needs) >> >> - RepositorySyncURL: >> { >> - required: >> [ >> - "repository" >> ], >> - type: "object", >> - properties: >> { >> - repository: >> { >> - title: "Repository", >> - description: "A URI of the repository to be synchronized.", >> - type: "string", >> - format: "uri" >> } >> } >> }, >> >> >> I can see why some users would want to refer to things in the api using >>> ID not an href. I think about the case that when calling publish and >>> referring to a RepositoryVersion with id=827561, num=3, and for >>> repository=1234. With an ID alternately accepted, you could call publish >>> and submit repo_version=827561 instead of >>> repo_version='repositories/1234/version/3/'. I can see that benefit, but it >>> comes with downsides. Saving/storing a url I know feels a little strange, >>> but I do see several upsides... >>> >> >>> Doing it only with hrefs, ensures those benefits (nice recap btw) will >>> always be true. Having to submit the references using something like >>> 'repositories/1234/version/3/' will cause any client to store them that >>> way. I think that's a good thing because someone troubleshooting their >>> scripts or in katello's db will instead >>> have 'repositories/1234/version/3/', which they can directly use with HTTP. >>> I think this is valuable. Otherwise they would have repo version 827561, >>> which now they have to do extra work to start interacting with that object >>> via HTTP. Storing urls removes the "templating" step from the >>> troubleshooter's responsibilities so we're making their job easier. >>> Spacewise, I don't think the clients benefit hugely from storing 827561 >>> instead of 'repositories/1234/version/3/', but humans do. >>> >> Why don't we provide the ability to use both href and id as identifiers, >> and katello can choose the route that is right for them based on the points >> you bring up? >> >>> >>> I don't know much about the CLI, but if we want to enable a specific >>> user experience, I think we can find a way to make that work. Overall I >>> think users should be able to specify things in the most intuitive way >>> possible, and I don't see how API data formats directly influence that. For >>> example I think referring to a repository by it's name is the most natural; >>> it's more natural than 1234 or repositories/1234. >>> >> +1 the CLI can resolve name to identifiers (either id or href), so I'm >> not too concerned with that. >> >> [0] >> https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types >> >> On Mon, Jul 23, 2018 at 9:51 PM, Dennis Kliban <dkli...@redhat.com> >> wrote: >> >>> I've made a work in progress PR[0] that demonstrates the changes I was >>> suggesting. >>> >>> [0] https://github.com/pulp/pulp/pull/3561 >>> >>> On Mon, Jul 23, 2018 at 3:50 PM, Brian Bouterse <bbout...@redhat.com> >>> wrote: >>> >>>> Having two ways to refer to objects in the API makes me nervous. I have >>>> some questions/concerns/ideas. I'm also interested to see what dkliban's >>>> bindings produce in terms of a resolution of the swagger issues. >>>> >>>> Won't it be problematic with the openapi definitions causing us to have >>>> two schemas? Accepting the data in two forms is one thing, but using >>>> openapi to describe it both ways is something I don't understand well. Are >>>> we going to ship and test two? >>>> >>>> I can see why some users would want to refer to things in the api using >>>> ID not an href. I think about the case that when calling publish and >>>> referring to a RepositoryVersion with id=827561, num=3, and for >>>> repository=1234. With an ID alternately accepted, you could call publish >>>> and submit repo_version=827561 instead of >>>> repo_version='repositories/1234/version/3/'. I can see that benefit, but it >>>> comes with downsides. Saving/storing a url I know feels a little strange, >>>> but I do see several upsides... >>>> >>>> Doing it only with hrefs, ensures those benefits (nice recap btw) will >>>> always be true. Having to submit the references using something like >>>> 'repositories/1234/version/3/' will cause any client to store them that >>>> way. I think that's a good thing because someone troubleshooting their >>>> scripts or in katello's db will instead >>>> have 'repositories/1234/version/3/', which they can directly use with HTTP. >>>> I think this is valuable. Otherwise they would have repo version 827561, >>>> which now they have to do extra work to start interacting with that object >>>> via HTTP. Storing urls removes the "templating" step from the >>>> troubleshooter's responsibilities so we're making their job easier. >>>> Spacewise, I don't think the clients benefit hugely from storing 827561 >>>> instead of 'repositories/1234/version/3/', but humans do. >>>> >>>> I don't know much about the CLI, but if we want to enable a specific >>>> user experience, I think we can find a way to make that work. Overall I >>>> think users should be able to specify things in the most intuitive way >>>> possible, and I don't see how API data formats directly influence that. For >>>> example I think referring to a repository by it's name is the most natural; >>>> it's more natural than 1234 or repositories/1234. >>>> >>>> >>>> On Thu, Jul 19, 2018 at 8:30 AM, Daniel Alley <dal...@redhat.com> >>>> wrote: >>>> >>>>> Keep in mind that as of yesterday, unless we revert the change, we are >>>>> using Integers IDs instead of UUIDs >>>>> >>>>> https://github.com/pulp/pulp/pull/3549 >>>>> >>>>> On Wed, Jul 18, 2018 at 9:57 PM, Bihan Zhang <bizh...@redhat.com> >>>>> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Wed, Jul 18, 2018 at 1:05 PM, Dennis Kliban <dkli...@redhat.com> >>>>>> wrote: >>>>>> >>>>>>> I was asked on IRC to state what problems the proposed changes are >>>>>>> trying to address. There are two problems I see with the current OpenAPI >>>>>>> 2.0 schema Pulp's REST API provides. >>>>>>> >>>>>>> - The path parameters in the schema don't reflect the parameters >>>>>>> our users should be using for identifying the resources available via >>>>>>> REST >>>>>>> API. >>>>>>> >>>>>> >>>>>> I'm not convinced that we should use hrefs as the sole identifiers >>>>>> for the resources. >>>>>> >>>>>> Here are the reasons I see that we use hrefs as identifiers in a REST >>>>>> API context: >>>>>> 1. Each href provides full context into the resource it >>>>>> identifies. When given a href you would know exactly which resource it is >>>>>> referencing and would never run into the issue of: what is this {uuid} >>>>>> because you know it is a 'repositories/{uuid}' >>>>>> 2. discoverability, you know exactly how to access resources from >>>>>> hitting the root url (and in a webui can just click) >>>>>> 3. You would not need to construct urls from templates >>>>>> >>>>>> But things are different if we look at it from a bindings/client >>>>>> context. The difference is mainly due to how discoverability is done: in >>>>>> the REST API context the user has little prior knowledge to what >>>>>> resources >>>>>> are available, and how to access theses resoruces. But the >>>>>> bindings/client >>>>>> are generated from the schema, which defines exactly how resources are >>>>>> structured, and what the context of each {uuid} is. >>>>>> >>>>>> 1. Given an {uuid} the client/bindings knows exactly what >>>>>> resource this {uuid} refers to. With hrefs there would be redundant >>>>>> information pulp.repositories('repositories/{uuid}') (why do I need to >>>>>> specify repositories twice?) >>>>>> 2. Discoverability is done with the schema which contains all the >>>>>> information about available resources/endpoints >>>>>> 3. URL construction is done by the client, so the user would also >>>>>> never need to do any url construction themselves (unless we continue to >>>>>> force href only identifiers, in which case they might have to do some url >>>>>> construction to pass as arguments) >>>>>> >>>>>> I don't think hrefs and uuid identifiers are mutually exclusive. I >>>>>> propose that we extend HyperlinkedRelatedFields to accept either href or >>>>>> uuid, and map these HyperlinkedRelatedFields to each other in the schema >>>>>> with openapi definition objects [0]. >>>>>> >>>>>> [0] >>>>>> https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#responses-definitions-object >>>>>> >>>>>> >>>>>>> >>>>>>> - The path parameters don't have a description in the schema. >>>>>>> >>>>>>> +1 to updating the schema descriptions for these parameters >>>>>> >>>>>> >>>>>>> Do others agree with these problem statements? >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Wed, Jul 18, 2018 at 9:31 AM, Dennis Kliban <dkli...@redhat.com> >>>>>>> wrote: >>>>>>> >>>>>>>> I am working on improving the OpenAPI 2.0 schema for Pulp 3. I >>>>>>>> would like to get some input on the improvements I am proposing. The >>>>>>>> schema >>>>>>>> is used to generate our REST API documentation as well as the bindings >>>>>>>> with >>>>>>>> swagger-codegen. >>>>>>>> >>>>>>>> The docs generated from our current schema look something like this: >>>>>>>> >>>>>>>> GET /repositories/{repository_pk}/versions/{number}/content/ >>>>>>>> <https://docs.pulpproject.org/en/3.0/nightly/integration-guide/rest-api/index.html#get--repositories-repository_pk-versions-number-content-> >>>>>>>> Parameters: >>>>>>>> >>>>>>>> - *number* (*integer*) – >>>>>>>> - *repository_pk* (*string*) – >>>>>>>> >>>>>>>> Status Codes: >>>>>>>> >>>>>>>> - 200 OK >>>>>>>> <http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.2.1> >>>>>>>> – >>>>>>>> >>>>>>>> >>>>>>>> Since Pulp identifies resources using their HREFs, I am proposing >>>>>>>> that the schema produce documentation that states: >>>>>>>> >>>>>>>> GET /{repository_version_href}/content/ >>>>>>>> Parameters: >>>>>>>> >>>>>>>> - *repository_version_href* (string) – HREF for the repository >>>>>>>> version >>>>>>>> >>>>>>>> Status Codes: >>>>>>>> >>>>>>>> - 200 OK >>>>>>>> <http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.2.1> >>>>>>>> – >>>>>>>> >>>>>>>> >>>>>>>> Thoughts? Ideas? All feedback is welcome. Thank you! >>>>>>>> >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> 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
