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> >
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
