[Pulp-dev] Proposal and feedback request: un-nest urls
Brian Bouterse
bbouters at redhat.com
Wed Nov 29 22:32:04 UTC 2017
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 <bizhang at 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 <jortel at 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" <pcreech at redhat.com <mailto:
>> pcreech at 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 at redhat.com <mailto:Pulp-dev at redhat.com>
>> > > > > https://www.redhat.com/mailman/listinfo/pulp-dev <
>> https://www.redhat.com/mailman/listinfo/pulp-dev>
>> > > > >
>> > >
>> > > _______________________________________________
>> > > Pulp-dev mailing list
>> > > Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
>> > > https://www.redhat.com/mailman/listinfo/pulp-dev <
>> https://www.redhat.com/mailman/listinfo/pulp-dev>
>> > _______________________________________________
>> > Pulp-dev mailing list
>> > Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
>> > https://www.redhat.com/mailman/listinfo/pulp-dev <
>> https://www.redhat.com/mailman/listinfo/pulp-dev>
>> >
>>
>>
>> _______________________________________________
>> Pulp-dev mailing list
>> Pulp-dev at redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
>>
>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20171129/5c3bfd32/attachment.htm>
More information about the Pulp-dev
mailing list