+1 to un-nesting for me as well.
David On Thu, Nov 30, 2017 at 8:48 AM, Dennis Kliban <dkli...@redhat.com> wrote: > +1 to not nesting > > I prefer the simplicity of unnested URLs for the API. This change will > require users to specify a repository href when creating an importer or a > publisher. This provides the same amount of information as a nested URL. > > On Wed, Nov 29, 2017 at 5:32 PM, Brian Bouterse <bbout...@redhat.com> > wrote: > >> For deletes, the db relationships are all there, so I expect deletes to >> cascade to other objects with any url structure. I believe closer to the >> release, we'll have to look at the cascading delete relationships to see if >> the behaviors that we have are correct. >> >> Overall, I'm +1 on un-nesting. I think it would result in a good user >> experience. I know it goes against the logical composition arguments, which >> have been well laid out. We want Pulp to be really simple, and the nested >> URL in the top of this thread is anything but simple. Consider another >> project like Ansible Galaxy (who also uses Django and DRF). Their API is >> very flat and as an outsider I find it very approachable: >> https://galaxy.ansible.com/api/v1/ Pulp could be that simple. >> >> My main concern in keeping the nesting is that this is going to be >> difficult for plugin writers. Making plugin writing easy is a primary goal >> if not the primary goal of Pulp3. If core devs are spending lots of time on >> it, a person doing this in their free time may not bother. >> >> I also see practical reasons motivating us to un-nest. We have been >> adding custom code regularly in this area, and it's been highly complexity >> and slow going. I think Austin described it well. Getting the viewsets >> working and to be simpler would allow us to move forward in many areas. >> >> So overall, un-nesting would give a better user experience (I think), a >> simpler plugin writer experience, and it would unblock a lot of work. >> >> >> >> On Wed, Nov 29, 2017 at 3:29 PM, Bihan Zhang <bizh...@redhat.com> wrote: >> >>> I have a question about repository delete with the un-nested model. >>> When a repository is deleted does the DELETE cascade to the >>> importers/publishers that are linked to the repo? In an un-nested world I >>> don't think they would. It would be odd for an object with its own endpoint >>> to vanish without the user calling DELETE on the model. >>> >>> When nested it makes sense to cascade the delete so if /repo/1/ is >>> deleted, everything thereafter (/repo/1/importer/2) should also be removed. >>> >>> Austin, I do see you point about it being a lot more complicated, but I >>> think modeling things the right way is worth carrying the extra code and >>> complexity. >>> >>> Anyways, maybe I'm wrong and importer/publishers should exist without a >>> repository, in which case I can definitely see the value in un-nesting the >>> URLs. >>> >>> >>> On Wed, Nov 29, 2017 at 2:21 PM, Jeff Ortel <jor...@redhat.com> wrote: >>> >>>> Austin makes a compelling argument. >>>> >>>> >>>> On 11/28/2017 02:16 PM, Austin Macdonald wrote: >>>> > When I look at this, the most important point is that we have a >>>> hyperlinked REST API, which means that the >>>> > urls are specifically not going to be built by users. >>>> > >>>> > For a user to retrieve an importer, they would first GET the >>>> importers for a repository. The next call would >>>> > be the exact href returned by pulp. This workflow is exactly the same >>>> whether we nest or not. The only >>>> > difference is that we no longer convey the information in the href, >>>> which seems fine to me since they aren't >>>> > particularly readable anyway. >>>> > >>>> > It has already been discussed that filtering can make up for the use >>>> cases that use nesting, and that filters >>>> > would be more flexible. >>>> > >>>> > So for me, nesting costs in (1) extra code to carry (2) extra >>>> dependency (3) complexity to use. >>>> > >>>> > To elaborate on the complexity, the problem is in declaring fields on >>>> the serializer. The serializer is >>>> > responsible for building the urls, which requires all of the uuids >>>> for the entire nested structure. This is >>>> > further complicated by master/detail, which is an entirely Pulp >>>> concept. >>>> > >>>> > Because of this, anyone working on the API (likely including plugin >>>> writers) will need to understand >>>> > parent_lookup_kwargs and how to use then with: >>>> > DetailNestedHyperlinkedRelatedField >>>> > DetailNestedHyperlinkedidentityField >>>> > DetailwritableNestedUrlRelatedField >>>> > DetailRelatedField >>>> > DetailIdentityField >>>> > NestedHyperlinkedRelatedField >>>> > HyperlinkedRelatedField. >>>> > >>>> > The complexity seems inherrent, so I doubt we will be able to >>>> simplify this much. So, is all this code and >>>> > complexity worth the implied relationship in non-human-friendly urls? >>>> As someone who has spent a lot of time >>>> > on this code, I don't think so. >>>> > >>>> > >>>> > >>>> > On Nov 28, 2017 06:12, "Patrick Creech" <pcre...@redhat.com <mailto: >>>> pcre...@redhat.com>> wrote: >>>> > >>>> > On Mon, 2017-11-27 at 16:10 -0600, Jeff Ortel wrote: >>>> > > On 11/27/2017 12:19 PM, Jeff Ortel wrote: >>>> > > > >>>> > > > >>>> > > > On 11/17/2017 08:55 AM, Patrick Creech wrote: >>>> > > > > One of the things I like to think about in these types of >>>> situations is, "what is good rest >>>> > > > > api >>>> > > > > design". Nesting resources under other resources is a >>>> necessary part of good api design, and >>>> > > > > has >>>> > > > > its place. To borrow some terms from domain driven >>>> development: >>>> > > > > >>>> > > > > Collections of objects are called aggregates. Think 'an >>>> order and its line items'. Line >>>> > > > > items make >>>> > > > > no sense without having the order context, so they are an >>>> aggregate that is accessed under an >>>> > > > > Order. This is called the aggregate root. The rest api >>>> design for such an object, using >>>> > > > > order as >>>> > > > > the aggregate root, would look like: >>>> > > > > >>>> > > > > '/orders/' -- all orders >>>> > > > > '/orders/{order_key}/' -- a specific order with key. >>>> > > > > '/orders/{order_key}/items/' -- All of the order's items. >>>> > > > > '/orders/{order_key}/items/{item_key}/' -- a specific line >>>> item of the order >>>> > > > > >>>> > > > > When it comes to order items themselves, it isn't helpful >>>> to start with them as their own >>>> > > > > aggregate >>>> > > > > root in one large collection: >>>> > > > > >>>> > > > > '/items/' -- all order items in the system >>>> > > > >>>> > > > The order/items is a good example of aggregation (or >>>> composition) and I agree it makes a strong >>>> > > > case for >>>> > > > nesting. In pulp, a repository is easily thought of as a >>>> collection or aggregation of content. >>>> > > > >>>> > > > > >>>> > > > > Because you lose the order context. Based on api design, >>>> this endpoint will need to respond >>>> > > > > with all >>>> > > > > order items across all orders and resort to parameter >>>> filtering to provide the context you >>>> > > > > need. >>>> > > > > >>>> > > > > A quote borrowed from Martin Fowler [0] >>>> > > > > >>>> > > > > "An aggregate will have one of its component objects be the >>>> aggregate root. Any references >>>> > > > > from >>>> > > > > outside the aggregate should only go to the aggregate root. >>>> The root can thus ensure the >>>> > > > > integrity >>>> > > > > of the aggregate as a whole." >>>> > > > > >>>> > > > > Publishers, importers, and publications are all aggregates >>>> that don't make much sense outside >>>> > > > > of >>>> > > > > their aggregate root of Repository. They are dependent on >>>> the Repository context, and from a >>>> > > > > domain >>>> > > > > view, should be accessed starting with their specific >>>> Repository endpoint. >>>> > > > >>>> > > > I don't think the aggregation relationship exists between >>>> repository and >>>> > > > importer/publisher. There is a >>>> > > > strong association between repository and importer/publisher >>>> which /could/ even be characterized >>>> > > > as >>>> > > > "ownership". However, I don't think there is an aggregation >>>> (or composition) relationship. The >>>> > > > same for >>>> > > > publisher & publication. A publication is associated to its >>>> creating publisher but the >>>> > > > publisher isn't an >>>> > > > aggregation of publications. The relationship mainly >>>> provides linkage to the repository. >>>> > > >>>> > > This is not an argument to flatten the URLs but meant to >>>> clarify the relationships. >>>> > >>>> > I'm in agreement here. I was possibly a little hasty in lumping >>>> all things that have a Repositoy fk >>>> > as being 'dependent' in that paragraph during the formation of my >>>> argument. >>>> > >>>> > > > >>>> > > > > >>>> > > > > ------------------------------ >>>> -------------------------------- >>>> > > > > Specific items rebuttals: >>>> > > > > >>>> > > > > Yes, using the primary key uuid's as the immutable key >>>> adds some human readable challenges >>>> > > > > to >>>> > > > > the API. That sounds more like a point to discuss in the >>>> human readable vs. not human >>>> > > > > readable >>>> > > > > immutable key debate. >>>> > > > >>>> > > > Agreed. >>>> > > > >>>> > > > Also, I don't think nesting impacts URL readability. >>>> > > > >>>> > > > > >>>> > > > > One of the challenges in software engineering is >>>> ensuring the tools you are using don't >>>> > > > > limit >>>> > > > > your choices. DRF limited the choices for pulp's rest API >>>> design, and drf-nested-routers was >>>> > > > > introduced to help remove that limit. If working around >>>> these limitations is complex, take >>>> > > > > advantage of open source here and help improve the upstream >>>> dependencies for your workflow. >>>> > > > > >>>> > > > > As far as making things simpler for plugin writers, >>>> perhaps there are ways you can >>>> > > > > simplify it >>>> > > > > for them by providing some encapsulation in pulp's core >>>> instead. Abstract away the nasty bits >>>> > > > > behind the scenes, and provide them with a simpler >>>> interface to do what they need. >>>> > > > > >>>> > > > > With respect to the invested time already in making >>>> this work, I agree with jeremy that it >>>> > > > > should be considered part of the sunken cost fallacy. What >>>> does need to be evaluated though >>>> > > > > is how >>>> > > > > much time re-architecting at this point will cost you >>>> (discussion, planning, and development) >>>> > > > > vs the >>>> > > > > amount of time it will save, and weigh that against any >>>> planned milestones for pulp to see if >>>> > > > > it >>>> > > > > will push them out as well. >>>> > > > > >>>> > > > > I'm also in agreement that it is moot if pulp3 has a >>>> different api structure than >>>> > > > > pulp2. Major >>>> > > > > version boundaries are the perfect time for evaluating and >>>> moving such things around. >>>> > > > > >>>> > > > > [0] https://martinfowler.com/bliki/DDD_Aggregate.html >>>> > <https://martinfowler.com/bliki/DDD_Aggregate.html> >>>> > > > > >>>> > > > > >>>> > > > > >>>> > > > > _______________________________________________ >>>> > > > > Pulp-dev mailing list >>>> > > > > Pulp-dev@redhat.com <mailto:Pulp-dev@redhat.com> >>>> > > > > https://www.redhat.com/mailman/listinfo/pulp-dev < >>>> https://www.redhat.com/mailman/listinfo/pulp-dev> >>>> > > > > >>>> > > >>>> > > _______________________________________________ >>>> > > Pulp-dev mailing list >>>> > > Pulp-dev@redhat.com <mailto:Pulp-dev@redhat.com> >>>> > > https://www.redhat.com/mailman/listinfo/pulp-dev < >>>> https://www.redhat.com/mailman/listinfo/pulp-dev> >>>> > _______________________________________________ >>>> > Pulp-dev mailing list >>>> > Pulp-dev@redhat.com <mailto:Pulp-dev@redhat.com> >>>> > https://www.redhat.com/mailman/listinfo/pulp-dev < >>>> 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