[Pulp-dev] Proposal and feedback request: un-nest urls
Daniel Alley
dalley at redhat.com
Thu Nov 30 18:11:36 UTC 2017
+0 to un-nesting
On Thu, Nov 30, 2017 at 11:19 AM, Bihan Zhang <bizhang at 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 <jortel at 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 <dkliban at redhat.com
>> <mailto:dkliban at 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 <
>> bbouters at redhat.com <mailto:bbouters at 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 <
>> bizhang at redhat.com <mailto: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 <mailto: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>
>> > <mailto: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>
>> > > <https://martinfowler.com/bli
>> ki/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> <mailto: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>
>> > <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>
>> <mailto: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>
>> > <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>
>> <mailto: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>
>> > <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 <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
>>
>>
>
> _______________________________________________
> 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/20171130/fb247ede/attachment.htm>
More information about the Pulp-dev
mailing list