Code

Opened 4 months ago

Closed 4 months ago

Last modified 2 months ago

#21595 closed Cleanup/optimization (wontfix)

Automatically call as_view() when urlpatterns encounter a CBV.

Reported by: loic84 Owned by:
Component: Core (URLs) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Calling as_view() for each CBV is not DRY and adds a fair amount of noise to urlpatterns.

I propose we check for the as_view attribute and automatically call it if present.

Attachments (0)

Change History (11)

comment:1 Changed 4 months ago by loic84

  • Has patch set
  • Needs documentation set
  • Needs tests unset
  • Owner changed from nobody to loic84
  • Patch needs improvement set
  • Status changed from new to assigned

Tentative patch: https://github.com/loic/django/compare/ticket21595.

I'll write the docs if this gets accepted.

comment:2 Changed 4 months ago by mjtamlyn

Not sure about this - I can see it being useful but I'm tentative to change the explicit api with something which is frankly magical. What I might find interesting is a cbv version of patterns which does clever things, but also perjhaos doesn't do the clever things the normal one does too handle string paths etc.

comment:3 Changed 4 months ago by aaugustin

Paraphrasing what I said on IRC yesterday:

  • This proposal breaks (extends) the contract that views are "just" callables; I'm quite attached to this design.
  • It introduces more than one way to do it".
  • It adds coupling between the URLresolver and the current implement of CBVs.

I'm -0.

comment:4 Changed 4 months ago by loic84

I see the as_view duck-type as an implementation agnostic hook into urlpatterns, one could for instance bring his own CBV implementation yet be compatible.

Regarding a "cbv version of patterns", that's something worth studying, but I think it would add a fair amont of complexity if we need to deal with different types of urlpatterns. In a way the pattern reminds me of https://github.com/jacobian/django-multiurl, which I really wanted handled inside core at the time.

I do share the "just callables" concern of @aaugustin; however, having embraced CBV, I can testify that my urls.py look pretty terrible at this point, so maybe it's one of those cases where practicality should beat purity?

comment:5 Changed 4 months ago by timo

@loic84 - do you want to write a quick email to the mailing list and try to get a consensus on this?

I tend to think -0 as well because "There should be one-- and preferably only one --obvious way to do it" and if you are using as_view() with parameters on some views, allowing things to be mixed isn't ideal. I'm sympathetic of the problem though.

comment:6 Changed 4 months ago by loic84

That would be the third -0 and I don't think I've received any positive feedback so far, so I don't think my implementation is worth pursuing.

If I understood properly @mjtamlyn mentioned on IRC that he wants to investigate the CBV urlpatterns idea; I guess we should keep this ticket open to track this effort.

comment:7 Changed 4 months ago by loic84

  • Owner loic84 deleted
  • Status changed from assigned to new

comment:8 Changed 4 months ago by timo

  • Resolution set to wontfix
  • Status changed from new to closed

Ok, let's open a new ticket for the CBV urlpatterns idea so it's not mixed with other conversation.

comment:9 Changed 2 months ago by Gwildor

I'm sorry for commenting on a closed ticket, but I can't seem to find a new ticket for this subject. Has a new ticket been opened yet?

My quick two cents on this subject: perhaps with the eye on the future the way the CBV's work should be changed, which I think is a backwards compatible change. At the moment, the CBV's actually function as FBV's in the urls, because the as_view() method, as the name states, returns a function which is then called with the url dispatching. My proposal: when url dispatching happens, call it as a FBV when it's a view (this way FBV's and CBV.as_view() are both still supported as they should). If it's a class, initialize and call dispatch() on it (__init__ should probably be rewritten for this, maybe dispatch can be called directly from the __init__ if it's a class). If it's a class instance, call dispatch() on it. This way, you can still pass initkwargs to the CBV in the urls, but the whole thing is more clear and generally makes more sense in my opinion. Python's inspect module can help with these things to check whether it's a function, a class or a class instance. A __call___ method on the View class might also come in handy here.

comment:10 Changed 2 months ago by russellm

This was considered and rejected as part of the original CBV work. See the wiki for the rationale.

Short version: it's all about keeping a simple contract for urlpatterns.

comment:11 Changed 2 months ago by mjtamlyn

For the sake of completeness, you may also be interested in this package: https://github.com/mjtamlyn/django-cbvpatterns

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.