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/bliki/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
