+0 to un-nesting On Thu, Nov 30, 2017 at 11:19 AM, Bihan Zhang <bizh...@redhat.com> wrote:
> After chatting with @asmacdo I am now +0 on this. > I've been convinced that treating importers, publishers, and content as > separate resources is a reasonable approach. > > On Thu, Nov 30, 2017 at 10:40 AM, Jeff Ortel <jor...@redhat.com> wrote: > >> +1 to flattening. >> >> On 11/30/2017 08:14 AM, David Davis wrote: >> > +1 to un-nesting for me as well. >> > >> > >> > David >> > >> > On Thu, Nov 30, 2017 at 8:48 AM, Dennis Kliban <dkli...@redhat.com >> <mailto: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 <mailto: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/ < >> 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 <mailto: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 <mailto: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> >> > <mailto: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> >> > > <https://martinfowler.com/bli >> ki/DDD_Aggregate.html >> > <https://martinfowler.com/bliki/DDD_Aggregate.html>> >> > > > > > >> > > > > > >> > > > > > >> > > > > > ______________________________ >> _________________ >> > > > > > Pulp-dev mailing list >> > > > > > Pulp-dev@redhat.com <mailto: >> Pulp-dev@redhat.com> <mailto: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> >> > <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> >> <mailto: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> >> > <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> >> <mailto: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> >> > <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 <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
