Opened 18 years ago

Closed 12 years ago

Last modified 5 years ago

#253 enhancement closed duplicate (duplicate)

Resource.render should allow returning a Deferred

Reported by: jknight Owned by:
Priority: normal Milestone:
Component: web Keywords:
Cc: itamarst, dp, jknight Branch:


Attachments (2)

render-deferred.patch.rtf (5.4 KB) - added by jknight 18 years ago.
render-deferred.patch (5.0 KB) - added by jknight 18 years ago.

Download all attachments as: .zip

Change History (23)

Changed 18 years ago by jknight

Attachment: render-deferred.patch.rtf added

comment:1 Changed 18 years ago by jknight

This patch fixes the API for Resource.render to allow you to return a 
Deferred, as one would expect to be able to do. This is as opposed to 
the currently recommended method of returning NOT_DONE_YET and 
restructuring your code to call request.write(...). Of course, that should 
still work, too.

Changed 18 years ago by jknight

Attachment: render-deferred.patch added

comment:2 Changed 18 years ago by jknight

bleargh, stupid RTF file.

comment:3 Changed 18 years ago by itamarst

There are a few places in the code that assume result of
render() is either NOT_DONE_YET or string. This is typically
done in wrapper resources that want to be transparent and
not alter request (e.g. my patch for improving
DeferredResource, or twisted.web.guard.) I suspect this
patch will break them.

comment:4 Changed 18 years ago by Moshe Zadka

I've removed the [PATCH] indicator, since the current patch 
isn't complete -- like itamar said, at least patch 

comment:5 Changed 18 years ago by itamarst

It's not just web.guard, it's other code in my own projects
and possibly others. I'm not sure if there is a solution :(
You can do stuff like having a reallyRender that render
calls and can return Deferreds which are converted to
NOT_DONE_YET, or... I dunno. Make __eq__ and friends for
NOT_DONE_YET do some magic for deferreds?

comment:6 Changed 18 years ago by jknight

Surely t.w.guard should be fixed (sorry didn't notice that problem). 
However, I'm not sure how much effort really needs to be put into being 
totally compatible with user code. Here is my proposed solution:
1) For the next X amount of time, do not return a deferred from render() 
in any code in the twisted framework (unless it's just passing it through 
a user's render()).
2) Make all code in twisted framework able to deal with user code 
returning a deferred.

With this solution, breakage can only occur if a user's code returns a 
deferred from render, *and* the user's code isn't able to deal with that. 
Nothing that currently works would break.

I think that's a reasonable compromise.

comment:7 Changed 18 years ago by itamarst

That seems like a reasonable solution, yes. Some sort of
utility code for wrapper resources so they don't have to
write the same code each time would also make it easier to
transition - "oh, just use this if you're doing that sort of

comment:8 Changed 18 years ago by Moshe Zadka

Can this be downgraded from "bug" to "feature"?

comment:9 Changed 18 years ago by Glyph

Somehow I feel as though 'bug' is the right classification, but at the same time
it's changing an existing interface which is ANCIENT in Twisted's time-scale
(which has problems as indicated).

jknight, can you update the patch to provide the policy that you suggest?  It
sounds good.

I'm assigning this to dp to make sure someone is in charge of following up and
getting this committed eventually.  I'd like it to be in 1.1 so that we can end
end "X amount of time" when we hit 1.2 and drop the other vestiges of ugly
backwards compatibility.

comment:10 Changed 18 years ago by jknight

I've tried to decipher the widget code in t.w.guard. The simple solution of just returning the 
subresource's render(..) isn't possible because I guess the return value of reallyRender needs to be 
widgets.FORGET_IT so that the first time after it authenticates you, it doesn't redisplay the login 

So, anyhow -- I *think* guard can be fixed by the following. It is somewhat odd as it calls back 
into the request's render method which you don't generally do, but it should work because the first 
render call aborted with NOT_DONE_YET and no headers have been sent yet. The test doesn't run 
because DummyRequest doesn't implement render. I don't see a simple example of t.w.guard use
and have no idea how all this set of authorizer/etc stuff works. 

So, this is untested but probably works. :)

@@ -210,10 +207,7 @@
     def reallyRender(self, request):
         # it's authenticated already...
         res = self.res.getChildForRequest(request)
-        val = res.render(request)
-        if val != NOT_DONE_YET:
-            request.write(val)
-            request.finish()
+        request.render(res)

comment:11 Changed 18 years ago by itamarst

This won't work for the case returns a string. Or for the case it returns a
Deferred ;)

comment:12 Changed 18 years ago by jknight

Hm? Explain? request.render doesn't return anything.

comment:13 Changed 18 years ago by itamarst

Good point! I thought you did resource.render. I'll check the request.render. If
it *doesn't* do what we want, we ought to add a method to Request that does :)

comment:14 Changed 18 years ago by Moshe Zadka

Re: testing.
The latest tests in show a way to use real requests, not dummy

comment:15 Changed 18 years ago by radix

Now that I've implemented that render_* change, allowing resources to return
deferreds from render_* should be easy: put the code to manage it in base

comment:16 Changed 17 years ago by radix

does nevow/twisted.web2 do this already?

comment:17 Changed 17 years ago by jknight

This bug is irrelevant now, people should use Nevow, or web2 whenever it's done to get this 
functionality. Closing wontfix.

comment:18 Changed 12 years ago by ghazel

Resolution: fixed
Status: closedreopened

twisted.web is alive and well. People very recently have been wondering why this wasn't ever improved:

Perhaps we should revisit this issue?

comment:19 Changed 12 years ago by Jean-Paul Calderone

Resolution: duplicate
Status: reopenedclosed

#3711 was a duplicate of this. Since a bunch of work has been started there, we'll keep this ticket closed as the duplicate though (whereas normally I would close the higher numbered ticket as the duplicate).

comment:20 Changed 11 years ago by <automation>

Owner: dp deleted

comment:21 Changed 5 years ago by GitHub <noreply@…>

In 177052b:

Error: Processor CommitTicketReference failed
 does not appear to be a Git repository. See the log for more information.
Note: See TracTickets for help on using tickets.