Closed GoogleCodeExporter closed 8 years ago
webapp2.WSGIApplication sets a request attribute that is used to build URLs.
There are tests with WSGIApplication.url_for() and RequestHandler.url_for()
working, so I'm confused with your report. Are you using
webapp2.WSGIApplication or the one from webapp? Please provide some more info.
-------
About Router.build arguments:
Router.build takes a request as argument (not response). The purpose is that
extended routes can use the request info to build URLs (say, a route that
builds an absolute URL or a URL for a subdomain).
Other routing systems don't take request as argument because they infer environ
variables in some other way to build absolute URLs (e.g., Routes reads
os.environ or uses information from middleware; werkzeug.routing is bound to a
Request or environ).
We are just being very explicit here because environ would need to read i some
way.
Alternatively it could take environ as argument or simply use os.environ; using
request is not bad in the context of webapp, imo.
Router.build is intended to be a "low level" API. Users should use
app.url_for() or RequestHandler.url_for() instead (both call builder passing
the current request object).
Original comment by rodrigo.moraes
on 10 Aug 2010 at 8:52
Sorry, replace response with request in the bug report. Unfortunately I cannot
edit it.
The way WSGIApplication.url_for is written works only in the context of
processing a request. If I create my application like so:
app = webapp2.WSGIApplication(...)
app.url_for(...)
This won't work. This is actually used when you are doing tests with WebTest.
You want to use url_for to generate the url to your app and then all them to
test their output.
Original comment by evlogimenos
on 11 Aug 2010 at 11:05
Hi.
Oh, it works with webtest, as demonstrated in our tests (we do have quite a few
url_for tests). Please see:
http://code.google.com/p/webapp-improved/source/browse/tests/test_routing.py
http://code.google.com/p/webapp-improved/source/browse/tests/test_handler.py
There are several strategies you can use. The simplest one is: just set a blank
Request in the app. webob.Request has an utility exactly for this. See:
app = webapp2.WSGIApplication(...)
app.request = Request.blank('/')
app.url_for(...)
Notice that Request is only needed when generating absolute URLs.
Original comment by rodrigo.moraes
on 12 Aug 2010 at 9:05
It works if you do app.request = None as well. The point is that this is
non-obvious. Either the documentation of url_for() should be changed to
specifically say that this function should not be called outside handling a
request or without explicitly setting application.request. Alternatively it
should be changed to not need the request.
Original comment by evlogimenos
on 12 Aug 2010 at 10:25
Sorry for the confusion. The purpose of having request is to be able to build
absolute URLs, and this is not something that impedes testing (it is actually
there to help testing), but a warning can be added.
I can also change it to not need a request object, but then os.environ will be
used to build absolute URLs. See that then in a testing environment you'd need
to set the environ variables that are expected in a HTTP request, or absolute
URLs could not be built. I think this makes testing more difficult (and is the
reason why request is passed explicitly).
Given these options, what you think is best?
A) Leave request argument, document its requirement in url_for().
B) Remove request argument, use os.environ to build absolute URLs.
C) make **environ** an optional argument (from request.environ), then use it
when available and fallback to os.environ when not (helps testing, makes it
possible to use build() out of a request).
Thanks for your feedback.
Original comment by rodrigo.moraes
on 13 Aug 2010 at 2:11
I would go for B and document it in url_for. You need to tamper with the
environ for all sorts of things for appengine testing to work anyway. You also
need to modify it to emulate logged in users/admins and whatnot during testing.
With that in mind modifying the environ is something you *have* to do to test
so its ok to need it for absolute urls as well.
Original comment by evlogimenos
on 13 Aug 2010 at 11:20
After asking for some advice about this issue, I decided to let request be
passed explicitly and added the following note to url_for() docs:
.. note::
This method, like :meth:`WSGIApplication.url_for`, needs the request
attribute to be set to build absolute URLs. This is because some
routes may need to retrieve information from the request to set the
URL host. We pass the request object explicitly instead of relying
on ``os.environ`` mainly for better testability, but it also helps
middleware.
Thanks for your feedback.
Original comment by rodrigo.moraes
on 17 Aug 2010 at 1:01
Original issue reported on code.google.com by
evlogimenos
on 8 Aug 2010 at 11:42