<div dir="auto">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.<div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">It has already been discussed that filtering can make up for the use cases that use nesting, and that filters would be more flexible.</div><div dir="auto"><br></div><div dir="auto">So for me, nesting costs in (1) extra code to carry (2) extra dependency (3) complexity to use.</div><div dir="auto"><br></div><div dir="auto">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. </div><div dir="auto"><br></div><div dir="auto">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:</div><div dir="auto"><span style="font-family:sans-serif">DetailNestedHyperlinkedRelatedField</span><br></div><div dir="auto"><span style="font-family:sans-serif">DetailNestedHyperlinkedidentityField</span><span style="font-family:sans-serif"><br></span></div><div dir="auto"><span style="font-family:sans-serif">DetailwritableNestedUrlRelatedField</span></div><div dir="auto"><font face="sans-serif">DetailRelatedField</font></div><div dir="auto"><font face="sans-serif">DetailIdentityField</font></div><div dir="auto"><font face="sans-serif">NestedHyperlinkedRelatedField</font></div><div dir="auto"><font face="sans-serif">HyperlinkedRelatedField.</font></div><div dir="auto"><font face="sans-serif"><br></font></div><div dir="auto"><font face="sans-serif">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.</font></div><div dir="auto"><font face="sans-serif"><br></font></div><div dir="auto"><font face="sans-serif"><br></font></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Nov 28, 2017 06:12, "Patrick Creech" <<a href="mailto:pcreech@redhat.com">pcreech@redhat.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, 2017-11-27 at 16:10 -0600, Jeff Ortel wrote:<br> > On 11/27/2017 12:19 PM, Jeff Ortel wrote:<br> > ><br> > ><br> > > On 11/17/2017 08:55 AM, Patrick Creech wrote:<br> > > > One of the things I like to think about in these types of situations is, "what is good rest<br> > > > api<br> > > > design". Nesting resources under other resources is a necessary part of good api design, and<br> > > > has<br> > > > its place. To borrow some terms from domain driven development:<br> > > ><br> > > > Collections of objects are called aggregates. Think 'an order and its line items'. Line<br> > > > items make<br> > > > no sense without having the order context, so they are an aggregate that is accessed under an<br> > > > Order. This is called the aggregate root. The rest api design for such an object, using<br> > > > order as<br> > > > the aggregate root, would look like:<br> > > ><br> > > > '/orders/' -- all orders<br> > > > '/orders/{order_key}/' -- a specific order with key.<br> > > > '/orders/{order_key}/items/' -- All of the order's items.<br> > > > '/orders/{order_key}/items/{<wbr>item_key}/' -- a specific line item of the order<br> > > ><br> > > > When it comes to order items themselves, it isn't helpful to start with them as their own<br> > > > aggregate<br> > > > root in one large collection:<br> > > ><br> > > > '/items/' -- all order items in the system<br> > ><br> > > The order/items is a good example of aggregation (or composition) and I agree it makes a strong<br> > > case for<br> > > nesting. In pulp, a repository is easily thought of as a collection or aggregation of content.<br> > ><br> > > ><br> > > > Because you lose the order context. Based on api design, this endpoint will need to respond<br> > > > with all<br> > > > order items across all orders and resort to parameter filtering to provide the context you<br> > > > need.<br> > > ><br> > > > A quote borrowed from Martin Fowler [0]<br> > > ><br> > > > "An aggregate will have one of its component objects be the aggregate root. Any references<br> > > > from<br> > > > outside the aggregate should only go to the aggregate root. The root can thus ensure the<br> > > > integrity<br> > > > of the aggregate as a whole."<br> > > ><br> > > > Publishers, importers, and publications are all aggregates that don't make much sense outside<br> > > > of<br> > > > their aggregate root of Repository. They are dependent on the Repository context, and from a<br> > > > domain<br> > > > view, should be accessed starting with their specific Repository endpoint.<br> > ><br> > > I don't think the aggregation relationship exists between repository and<br> > > importer/publisher. There is a<br> > > strong association between repository and importer/publisher which /could/ even be characterized<br> > > as<br> > > "ownership". However, I don't think there is an aggregation (or composition) relationship. The<br> > > same for<br> > > publisher & publication. A publication is associated to its creating publisher but the<br> > > publisher isn't an<br> > > aggregation of publications. The relationship mainly provides linkage to the repository.<br> ><br> > This is not an argument to flatten the URLs but meant to clarify the relationships.<br> <br> I'm in agreement here. I was possibly a little hasty in lumping all things that have a Repositoy fk<br> as being 'dependent' in that paragraph during the formation of my argument.<br> <br> > ><br> > > ><br> > > > ------------------------------<wbr>------------------------------<wbr>--<br> > > > Specific items rebuttals:<br> > > ><br> > > > Yes, using the primary key uuid's as the immutable key adds some human readable challenges<br> > > > to<br> > > > the API. That sounds more like a point to discuss in the human readable vs. not human<br> > > > readable<br> > > > immutable key debate.<br> > ><br> > > Agreed.<br> > ><br> > > Also, I don't think nesting impacts URL readability.<br> > ><br> > > ><br> > > > One of the challenges in software engineering is ensuring the tools you are using don't<br> > > > limit<br> > > > your choices. DRF limited the choices for pulp's rest API design, and drf-nested-routers was<br> > > > introduced to help remove that limit. If working around these limitations is complex, take<br> > > > advantage of open source here and help improve the upstream dependencies for your workflow.<br> > > ><br> > > > As far as making things simpler for plugin writers, perhaps there are ways you can<br> > > > simplify it<br> > > > for them by providing some encapsulation in pulp's core instead. Abstract away the nasty bits<br> > > > behind the scenes, and provide them with a simpler interface to do what they need.<br> > > ><br> > > > With respect to the invested time already in making this work, I agree with jeremy that it<br> > > > should be considered part of the sunken cost fallacy. What does need to be evaluated though<br> > > > is how<br> > > > much time re-architecting at this point will cost you (discussion, planning, and development)<br> > > > vs the<br> > > > amount of time it will save, and weigh that against any planned milestones for pulp to see if<br> > > > it<br> > > > will push them out as well.<br> > > ><br> > > > I'm also in agreement that it is moot if pulp3 has a different api structure than<br> > > > pulp2. Major<br> > > > version boundaries are the perfect time for evaluating and moving such things around.<br> > > ><br> > > > [0] <a href="https://martinfowler.com/bliki/DDD_Aggregate.html" rel="noreferrer" target="_blank">https://martinfowler.com/<wbr>bliki/DDD_Aggregate.html</a><br> > > ><br> > > ><br> > > ><br> > > > ______________________________<wbr>_________________<br> > > > Pulp-dev mailing list<br> > > > <a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a><br> > > > <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>mailman/listinfo/pulp-dev</a><br> > > ><br> ><br> > ______________________________<wbr>_________________<br> > Pulp-dev mailing list<br> > <a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a><br> > <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>mailman/listinfo/pulp-dev</a><br>______________________________<wbr>_________________<br> Pulp-dev mailing list<br> <a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a><br> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>mailman/listinfo/pulp-dev</a><br> <br></blockquote></div></div>