Adding "abort" and "redirect" to Pyramid
Chris McDonough
contains an implementation of "redirect" and "abort" for Pyramid that
act like their Pylons brethren.
In short, the abort feature is used like this:
from pryamid.httpexceptions import abort
def aview(request):
abort(401)
This will perform the same job as what used to be necessary as:
def aview(request):
return HTTPUnauthorized()
The redirect feature is used like this:
from pryamid.httpexceptions import redirect
def aview(request):
redirect('http://example.com')
This will perform the same job as what used to be necessary as:
def aview(request):
return HTTPFound(location='http://example.com')
In addition, any "HTTP exception" (an exception imported from
pyramid.httpexceptions) can now be raised rather than returned. The
object raised will be used as a response.
Here's the branch:
https://github.com/Pylons/pyramid/tree/httpexception-utils
Here's the commit which added the feature:
https://github.com/Pylons/pyramid/commit/1ffb8e3cc21603b29ccd78152f82cca7f61a09b1
It'd be nice to get some feedback from existing Pylons users to see if
the implementations of these features are "good enough"; it'd also be
nice to hear dissenting opinions with reasons for dissent if folks
believe this feature should not be added to Pyramid.
- C
Wichert Akkerman
> I've created a branch named "httpexception-utils" on GitHub which
> contains an implementation of "redirect" and "abort" for Pyramid that
> act like their Pylons brethren.
>
> In short, the abort feature is used like this:
>
> from pryamid.httpexceptions import abort
>
> def aview(request):
> abort(401)
>
> This will perform the same job as what used to be necessary as:
>
> def aview(request):
> return HTTPUnauthorized()
-1
I find "return HTTPUnauthorized()" to be immensely more readable then
"abort(401)", so for me that is a step backwards.
> The redirect feature is used like this:
>
> from pryamid.httpexceptions import redirect
>
> def aview(request):
> redirect('http://example.com')
>
> This will perform the same job as what used to be necessary as:
>
> def aview(request):
> return HTTPFound(location='http://example.com')
-0
Readability of these is similar, so no objections for that reason. Here
I prefer returning a response for consistency: you also return responses
for Forbidden, NotFound and Unauthorized to it makes sense to use a
response here as well.
> In addition, any "HTTP exception" (an exception imported from
> pyramid.httpexceptions) can now be raised rather than returned. The
> object raised will be used as a response.
+0
+1 if that is also supported for context factories.
Wichert.
--
Wichert Akkerman <wic...@wiggy.net> It is simple to make things.
http://www.wiggy.net/ It is hard to make things simple.
Michael Merickel
Chris McDonough
> Is there any support for integrating "abort(404)" with "raise
> NotFound" such that my 404 is rendered properly? Same with
> "abort(403)" and Forbidden.
No, except in the docs I explain the difference between NotFound and
HTTPNotFound and Forbidden and HTTPForbidden. I'm really loath to try
to marry them together in some unholy way.
- C
>
>
> I'm a little torn but more in favor of the readability of using
> httpexceptions directly.
>
> --
> You received this message because you are subscribed to the Google
> Groups "pylons-devel" group.
> To post to this group, send email to pylons...@googlegroups.com.
> To unsubscribe from this group, send email to pylons-devel
> +unsub...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/pylons-devel?hl=en.
Tres Seaver
Hash: SHA1
+1 to the 'abort()' and 'redirect()' APIs.
- -1 to added non-local majyk in the guts of the publisher. I think the
use case is better served by adding an exception-mapping decorator to
the funcitons which might raise. Something like::
from webob.exc import HTTPException
def returnRaised(wrapped):
def _wrapper(*argv, **kw):
try:
return wrapped(*argv, **kw)
except HTTPException, e:
return e
return _wrapper
View code would look like::
@returnRaised
def aview(request):
abort(401)
Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 tse...@palladion.com
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk3RZbQACgkQ+gerLs4ltQ4TVgCfY6peYYiFAK3c+Gk/wIH6gynh
g0IAoJjORRn9PCgF5gH0U2Q93ZjBtczr
=LecC
-----END PGP SIGNATURE-----
Chris Withers
> On 05/16/2011 02:27 AM, Chris McDonough wrote:
>> I've created a branch named "httpexception-utils" on GitHub which
>> contains an implementation of "redirect" and "abort" for Pyramid that
>> act like their Pylons brethren.
>>
>> In short, the abort feature is used like this:
>>
>> from pryamid.httpexceptions import abort
>>
>> def aview(request):
>> abort(401)
>>
>> This will perform the same job as what used to be necessary as:
>>
>> def aview(request):
>> return HTTPUnauthorized()
I'd be +1 on making there be one object that could be returned or raised
for each of NotFound, Forbidden, etc, but I guess I'm unaware of the
holy way that would need to be waged.
I'm 0 on the short-form functions above that raise or return these (I
believe the code above has a bug - should the return be a raise in the
definition of aview?)
> View code would look like::
>
> @returnRaised
> def aview(request):
> abort(401)
I'm a big -1 on this though. Less decorators the better...
"Why do I have to decorate something just to get what should be default
behaviour?"
"I have to decorate every single one of my view methods?!"
Not my specific views, but I can see many people having them...
Chris
--
Simplistix - Content Management, Batch Processing & Python Consulting
- http://www.simplistix.co.uk
Stephen Lacy
Chris, what's the root issue that made you think that "return abort(401)" would be a better API than "return HTTPUnauthorized(...)"? What specific problem are you trying to solve? Is it just Pylons compatibility? How about making a utils or library specifically for that task, and making it really clear that there's a "Pyramid way" and a "Pylons compatibility way"? (Again, this violates the Zen, but at least it's a bit more explicit)
--
You received this message because you are subscribed to the Google Groups "pylons-devel" group.
To post to this group, send email to pylons...@googlegroups.com.
To unsubscribe from this group, send email to pylons-devel...@googlegroups.com.
Michael Merickel
Chris Withers
> design. I'm not fond of thinking of things like access violations as
> "exceptions".
Oh, I very much am. It's much easier and more powerful to, at any point,
raise a "whoa, you're not allowed to do that!" exception, and not have
to worry about all the intervening code "doing the right thing" - such
as not modifying the database, when that happens...
cheers,
Chris Withers
> I really don't like the idea of the abort(404) API providing no way to
> provide a custom 404 page,
I don't think anything Chris has said suggests you wouldn't be able to
provide a customer 404 page...
> that's why I mention marrying the two. And
> calling it abort(404) in no way implies that this is different from
> NotFound.
...indeed, and in both cases, you'd just be registering a view for the
NotFound exception.
cheers,
Chris McDonough
> I've created a branch named "httpexception-utils" on GitHub which
> contains an implementation of "redirect" and "abort" for Pyramid that
> act like their Pylons brethren.
Thanks a lot for everybody's input on this.
There was pushback on both the abort() and redirect() APIs and on the
registration of new default exception views.
Steve Lacy: there is no equivalence between "return abort(401)" and
"return HTTPNotFound()". Instead there is an equivalence between
"abort(401)" and "raise HTTPNotFound()". Pyramid already has a feature
which allows a developer to convert exceptions to responses
( http://docs.pylonsproject.org/projects/pyramid/1.0/narr/views.html#exception-views ), the "abort()" and "redirect()" spellings would just be Pylons-esque candy on top of this. It sounds like you're objecting to both the exception view feature and to the candy and you'd prefer to always just return a response, I think.
Tres: Given that we already have exception view machinery (the magic you
refer to already exists), I don't think there would be much purpose in
adding a decorator that converts an exception to a response instead of
just registering exception views.
Mike M.: The fact that abort(404) (or raise HTTPNotFound) would not
invoke the not found view is a genuine concern. This is the primary
reason I haven't just gone ahead and merged the branch. I'm not really
sure how to make this happen in a way that I'm confident doesn't paint
us into a corner in the future, though.
Bleh. Not really sure what to do.
- C
Alice Bevan–McGregor
Apologies if this post sounds too "absolutist", but I see this as an
issue that was resolved many years ago.
The way I see it there are three options:
:: Raise an exception from a pool of available exceptions.
:: Return an object, possibly an instance of something.
:: Execute a function which either internally re-routes or raises an exception.
Let's examine the pros/cons of each, in reverse order. First, functions:
+ Simple to document.
+ Simple to use.
- Non-obvious. Developers have to be fluent in HTTP status codes.
- Difficult to customize. (Middleware or API.)
- Magic; only core developers sure what really goes on inside.
- If handled using exceptions internally it arbitrarily increases the stack.
- AKA "Should just use the damn exceptions directly." ;)
- Likely complicated to implement. (PEP-20)
Returns:
~ Not entirely sure what the benefit(s) would be.
- Multiple objects to document.
- Possibly multiple calling syntaxes (e.g. notfound, Redirect(foo))
- Have to manually handle if returned from a nested call.
Exceptions:
+ Bubbles, so no return value checking.
+ Already fully documented and well implemented as part of WebOb.
+ Easily customizable through subclassing.
+ Somewhat self-documenting via the exception names.
- Magic to new developers unfamiliar with exceptions.
On 2011-05-16 11:42:11 -0700, Stephen Lacy said:
> "There should be one-- and preferably only one --obvious way to do it."
Amen.
> In general, I find the "returns or raises" pattern to be a fairly weird
> design. I'm not fond of thinking of things like access violations as
> "exceptions".
Think of it this way: if it walks like an exception (you want it to
bubble), talks like an exception (the vast majority of HTTP status
codes are in the 4xx and 5xx range; errors), then it should probably be
an exception. Your application has reached an exceptional (non-200)
state. ;)
> I'd like to be able to read my view function and know exactly what HTTP
> return codes are going to be produced.
It's a fairly simple project-wide, module-wide, or function/method-wide
search: "raise HTTP".
> When some function way down the call stack can raise up an arbitrary
> exception from httpexceptions, it makes it really unclear what my view
> is going to actually do.
Well, the general policy for exceptions are handle the ones you expect
and pass along the ones you don't. (E.g. a bare "except:" block is
bad, bad mojo.) I'd consider that true even for 3xx, 4xx, and 5xx
status codes. Additionally, that's what finally: and else: blocks are
for.
The possible exceptions a reusable block of code can raise should be
part of the documentation of that code the same as the argspec or
expected return values are.
— Alice.
Mike Orr
Raising HTTP exceptions should definitely be added to Pyramid. It's
silly for an application to have to add an exception view that just
returns the exception: it feels kludgy, it's not what views are for,
and it adds to the overhead. Why can't the code that invokes the view
just have an 'except HTTPException:'? If the user really wants to
register an exception view in order to display a fancy-dancy page,
that's another thing. It seems like Pyramid should be able to
accommodate both.
HTTP errors *are* exceptions. If you call a support method and it
discovers that a required query parameter is missing, what's it
supposed to do? The request can't continue, so it might as well kill
it right there. That's directly akin to a ZeroDivisionError. 'raise'
has two advantages. One, it shortcuts the call stack so you don't have
to return some dummy value, or define another exception just to catch
it later. Two, 'raise' is a Python keyword so it's syntax-highlighted
and users should be expecting it.
abort() and redirect() are not necessarily the most intuitive but I
can't think of any better API for them. I did use them a lot in Pylons
and I miss them a bit in Pyramid. They do have the disadvantage that
they look like normal returning function calls instead of having that
'raise' keyword. 'redirect' is particularly useful because it's not
intuitive that 'HTTPFound' means "I'm doing a redirect". If you see it
often you get used to it, but otherwise it's like, "Oh, nice, it found
something. That doesn't tell me what it's going to do with it." I
think that's a shortcoming of the HTTP status: it should have been
called 'Redirect' rather than 'Found'.
BTW, I mentioned a few days ago that my application is displaying a
blank page when I return HTTPNotFound or HTTPBadRequest, so something
is missing somewhere. The HTTP status is right but the body is empty,
no "Not Found" or anything.
--
Mike Orr <slugg...@gmail.com>
Daniel Holth
In Pyramid you could replace the standard context finding methods with a view that raises exceptions and build your whole application out of exception views.
Chris Withers
> In Pyramid you could replace the standard context finding methods with a
> view that raises exceptions and build your whole application out of
> exception views.
...which result in the PSU (That's *not* the Pyramid Secret Underground,
which doesn't exist and certainly isn't staffed by blood-hungry mummies)
in hunting you down and hanging your head on their belt, next to the
pink pony head ;-)
Mike Orr
> 'return' makes sense because views return a response. Whether the response
> has an error code of 200, 301 or 5xx is a separate concern. Of course
> exceptions make sense too.
This only became an issue because HTTPException happens to be a WSGi
application so it looks like a view return value. It would have been
better if it were just an Exception subclass so there would be no
question they should be raised and caught.
In fact, I'm not even sure the body and headers of the HTTPException
should be honored; it's really the job of the framework to decide how
to display HTTP errors (possibly using a plugin such as an error-view
to customize it). in Pylons, if the StatusCodeRedirect middleware is
active, it makes a subrequest. If it's not active, something in Pylons
or Paste generates a plain error message including the title,
description, and (in debug mode) the exception's 'message' argument.
It's certainly not the responsibility of the code that discovers an
inconsistency necessitating a 4xx or 5xx error to style it: the error
is rarely the main purpose of the function; it's just something the
function wants to get rid of as quickly as possible.
If the view really wants to RETURN a 4xx or 5xx condition, it can set
'request.response_status_int' the normal way rather than invoking
HTTPException. Or it can instantiate the exception and pass it to an
application-making function that would turn it into a WSGI application
(something like the HTTPException constructor), and return that.
--
Mike Orr <slugg...@gmail.com>
Joe Dallago
personally will continue to use the old style of http.exceptions, I
think this addition will help people coming from other frameworks, as
the "redirect" and "abort" keywords are commonplace for such methods
on other frameworks.
Chris McDonough
> I've created a branch named "httpexception-utils" on GitHub which
> contains an implementation of "redirect" and "abort" for Pyramid that
> act like their Pylons brethren.
- An exception view for the pyramid.interfaces.IExceptionResponse
interface will be registed by default. This exception view will
simply return the response object it receives as an exception.
It will be possible to disuse this default by passing a None value
to a constructor parameter. It will be possible to override the
default by passing a different view callable to the same constructor
parameter.
- All objects that inherit from pyramid.response.Response (inlcuding
instances of the Response class itself) will provide the
IExceptionResponse interface.
- We will disuse the classes from webob.exc because, although they
advertise themselves as Response objects, they really very badly
want to be used as WSGI applications rather than "plain response"
objects. We will create a mirror of the http exception hierarchy
in pyramid.response. It will be backwards compatible with the
current
crop of classes that are in "webob.exc" (aka
"pyramid.httpexceptions").
A backwards compatibility shim for "pyramid.httpexceptions" will be
left
in place.
- "pyramid.response.Response" will now be a *subclass* of
webob.response.Response (rather than an alias) which will
both inherit from Exception (so it can be raised) and will provide
the pyramid.interfaces.IExceptionResponse interface.
- the abort() and redirect() candy will be added. These will simply
raise exceptions.
After the above steps are taken, "raise
pyramid.response.HTTPUnauthorized(...)" from within view code (or
event handler code) will generate a 401 response code with a default
body out-of-the-box. It will mean (probably more controversially)
"raise Response('OK')" will generate a 200 response code with the body
"OK". This change should not cause much (if any) existing code to
break and if it does, the feature which provides this default behavior
can be turned off at Configurator constructor time.
After spending some time investigating alternative solutions, and
attempting to write documentation in a number of ways, allowing all
objects that inherit from Response to be raised seemed to be the most
sensible default as a principal of least surprise.
Mike Orr
> - We will disuse the classes from webob.exc because, although they
> advertise themselves as Response objects, they really very badly
> want to be used as WSGI applications rather than "plain response"
> objects.
Aren't all Response objects WSGI applications? I thought that was part
of the API, that they could serialize themselves.
> - "pyramid.response.Response" will now be a *subclass* of
> webob.response.Response (rather than an alias) which will
> both inherit from Exception (so it can be raised) and will provide
> the pyramid.interfaces.IExceptionResponse interface.
Can't anything be raised regardless of whether it inherits from
Exception? (And for new-style classes, if the Python version is recent
enough.) I think Pylons' abort() raises HTTPException subclasses,
doesn't it?
So I don't see how these new exception classes will be different from
the old ones except for a new parent and interface, which doesn't
really affect the user.
> After the above steps are taken, "raise
> pyramid.response.HTTPUnauthorized(...)" from within view code (or
> event handler code) will generate a 401 response code with a default
> body out-of-the-box. It will mean (probably more controversially)
> "raise Response('OK')" will generate a 200 response code with the body
> "OK".
It may be an unorthodox way to return but it's probably better to just
allow it than to take steps to prevent it. I could see how it could be
a "feature" in a few cases. And again, Python doesn't seem to be
overly concerned with what you raise. The move against string
exceptions was to make sure you provided the actual class in the
'except' clause rather than comparing by equality.
--
Mike Orr <slugg...@gmail.com>
Chris McDonough
> On Tue, May 31, 2011 at 12:55 PM, Chris McDonough <chr...@plope.com> wrote:
> > - We will disuse the classes from webob.exc because, although they
> > advertise themselves as Response objects, they really very badly
> > want to be used as WSGI applications rather than "plain response"
> > objects.
>
> Aren't all Response objects WSGI applications? I thought that was part
> of the API, that they could serialize themselves.
Pyramid only requires that objects used as responses have three
attributes: app_iter, headerlist, status. They do not have to inherit
from Response or any other bas class.
> > - "pyramid.response.Response" will now be a *subclass* of
> > webob.response.Response (rather than an alias) which will
> > both inherit from Exception (so it can be raised) and will provide
> > the pyramid.interfaces.IExceptionResponse interface.
>
> Can't anything be raised regardless of whether it inherits from
> Exception? (And for new-style classes, if the Python version is recent
> enough.)
Nope.
Chris McDonough
On Tue, 2011-05-31 at 14:47 -0700, Mike Orr wrote:
> On Tue, May 31, 2011 at 12:55 PM, Chris McDonough <chr...@plope.com> wrote:
> > - We will disuse the classes from webob.exc because, although they
> > advertise themselves as Response objects, they really very badly
> > want to be used as WSGI applications rather than "plain response"
> > objects.
>
> Aren't all Response objects WSGI applications? I thought that was part
> of the API, that they could serialize themselves.
Pyramid only requires that objects used as responses have three
attributes: app_iter, headerlist, status. They do not have to inherit
from Response or any other base class.
> > - "pyramid.response.Response" will now be a *subclass* of
> > webob.response.Response (rather than an alias) which will
> > both inherit from Exception (so it can be raised) and will provide
> > the pyramid.interfaces.IExceptionResponse interface.
>
> Can't anything be raised regardless of whether it inherits from
> Exception? (And for new-style classes, if the Python version is recent
> enough.)
Nope.
[chrism@thinko droidbuild]$ python2.6
Python 2.6.5 (r265:79063, Apr 29 2010, 00:31:32)
[GCC 4.4.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> raise object
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: exceptions must be old-style classes or derived from BaseException, not type
> I think Pylons' abort() raises HTTPException subclasses,
> doesn't it?
Probably.
>
> So I don't see how these new exception classes will be different from
> the old ones except for a new parent and interface, which doesn't
> really affect the user.
The things mentioned above (and in the original email) conspire to make it necessary to replace them.
It's no great loss to need to replace them, to be honest.
Philip Jenvey
I don't see why that's enough of a reason to throw the webob.exc exceptions away, why's the WSGI app compatibility such a problem?
> - "pyramid.response.Response" will now be a *subclass* of
> webob.response.Response (rather than an alias) which will
> both inherit from Exception (so it can be raised) and will provide
> the pyramid.interfaces.IExceptionResponse interface.
Is there a way to make the IExceptionResponse hook without making Response become a subclass of Exception?
--
Philip Jenvey
Chris McDonough
webob.exc HTTP* classes use templates that expect to have the request
environ available to resolve (non-optional) values. For example,
HTTPMethodNotAllowed wants to get request.environ['REQUEST_METHOD'] (to
be able to show "Request method GET is not allowed" as opposed to
"Request method is not allowed"). Currently, people can return to
Pyramid a Response object that has no reference to the originating
request. If we want to use webob.exc responses (as opposed to creating
our own), users will have to return responses with enough info to
resolve these sorts of values. In order to do that, we'd have to do one
of the following:
- Make people pass the request into the response's constructor.
- Change the contract of what Pyramid considers a "valid response" (aka
pyramid.interfaces.IResponse to) obligate it to include a method
that effectively turns it into a WSGI application.
I'm not real keen on the former because it's too much of a pain in the
ass.
I'm not real keen on the latter because requiring it will be a backwards
compatibility foul and not requiring it will require the router to
always do a guessy duck-check on every response rendering.
But of the two, the latter is less onerous.
> > - "pyramid.response.Response" will now be a *subclass* of
> > webob.response.Response (rather than an alias) which will
> > both inherit from Exception (so it can be raised) and will provide
> > the pyramid.interfaces.IExceptionResponse interface.
>
> Is there a way to make the IExceptionResponse hook without making Response become a subclass of Exception?
That's not really the problem. The problem is Python disallowing the
raising of new-style classes that don't inherit from BaseException as
exception objects. Unless a Response inherits from BaseException,
nobody will be able to raise one.
- C
Alexandre Conrad
This post totally went over my head and there's a lot to read which I
don't feel like doing right now, so I'll just give my 2 cent straight
away (and if it was already addressed, reply "already addressed", and
I will dig into the thread's archives):
2011/5/15 Chris McDonough <chr...@plope.com>:
> def aview(request):
> abort(401)
>
> def aview(request):
> redirect('http://example.com')
What I *really* didn't like with Pylons abort() and redirect() was the
fact that that these functions were stopping the code flow without a
return statement. And it always freaked me out. :) I think it raised
an exception that was being caught by an underlying library, which
re-wrapped the exception in a http response. It looked wrong by design
from my point of view.
Because the above views don't explicitly return, they should
implicitly return None IMO.
I'd rather have an explicit return:
def aview(request):
return redirect("http://example.com")
We may also be able to do something like:
def aview(request):
response = redirect("http://example.com")
# do something special with response
return response
Bleh.
--
Alex | twitter.com/alexconrad
Mike Orr
<alexandr...@gmail.com> wrote:
> Hi,
>
> This post totally went over my head and there's a lot to read which I
> don't feel like doing right now, so I'll just give my 2 cent straight
> away (and if it was already addressed, reply "already addressed", and
> I will dig into the thread's archives):
>
> 2011/5/15 Chris McDonough <chr...@plope.com>:
>> def aview(request):
>> abort(401)
>>
>> def aview(request):
>> redirect('http://example.com')
>
> What I *really* didn't like with Pylons abort() and redirect() was the
> fact that that these functions were stopping the code flow without a
> return statement.
We could just make abort() and redirect() helper functions that return
an exception, which the user would then raise. The priority for me is:
1. Make HTTPExceptions raisable by default.
2. Add a helper to construct a redirect:
redirect(location, **kw) => HTTPFound(location=location, **kw)
3. Add a helper to construct an abort:
abort(N, message=None, **kw) =>
httpexception_by_number(status=N, message=message, **kw)
4. Make abort() and redirect() raise their result rather than returning it.
#1 is important so that you can cut through multiple function calls if
you discover an error condition.
Use case 1: a support method/function for several handlers
discovers a required query parameter is missing. The application's
forms and links would never produce such a request, so we presume the
request is illegitimate and abort 400.
Use case 2: a support method/function discovers that a required
support file is missing, an authentication server is unreachable, etc.
This is a webmaster/sysadmin error so we abort 500.
#2 is important because HTTPFound is not very self-documenting, and we
shouldn't have to specify the location by keyword argument since it's
the entire purpose of the call.
#3 is useful but perhaps not necessary. It's convenient but not
necessary self-documenting because you have to memorize all the
numbers. It's also helpful to have a message as a positional argument.
This would be added to the error message. This has been proven useful
in Pylons. A further enhancement would be to have both a secure and an
insecure message. The secure message would appear in the default error
screen, while the insecure message would be included in the sysadmin's
error report.
#4 is minimally useful. They do have the significant downside of
looking like returning function calls when they actually change the
program flow. It would be fine if they simply return the error and the
user raise it explicitly.
raise redirect(location)
raise abort(N, message=None)
Or they could be capitalized to appear more like class constructors:
raise Redirect(location)
raise Abort(N, message=None)
--
Mike Orr <slugg...@gmail.com>
Chris McDonough
> On Tue, May 31, 2011 at 11:21 PM, Alexandre Conrad
> <alexandr...@gmail.com> wrote:
> > Hi,
> >
> > This post totally went over my head and there's a lot to read which I
> > don't feel like doing right now, so I'll just give my 2 cent straight
> > away (and if it was already addressed, reply "already addressed", and
> > I will dig into the thread's archives):
> >
> > 2011/5/15 Chris McDonough <chr...@plope.com>:
> >> def aview(request):
> >> abort(401)
> >>
> >> def aview(request):
> >> redirect('http://example.com')
> >
> > What I *really* didn't like with Pylons abort() and redirect() was the
> > fact that that these functions were stopping the code flow without a
> > return statement.
>
> We could just make abort() and redirect() helper functions that return
> an exception, which the user would then raise. The priority for me is:
>
> 1. Make HTTPExceptions raisable by default.
FTR, HTTPExceptions are and always have been raisable, they're just not
currently caught by default. Sorry if making that distinction sounds a
bit pedantic, but important because this is exactly what a developer
needs to do in order to catch and display them currently:
from pyramid.httpexceptions import HTTPException
config.add_view(lambda context, request: context,
context=HTTPException)
In other words, the argument boils down to whether to make people add
the above to their application configuration or whether Pyramid does it
on their behalf by default.
> #1 is important so that you can cut through multiple function calls
if
> you discover an error condition.
> Use case 1: a support method/function for several handlers
> discovers a required query parameter is missing. The application's
> forms and links would never produce such a request, so we presume the
> request is illegitimate and abort 400.
> Use case 2: a support method/function discovers that a required
> support file is missing, an authentication server is unreachable, etc.
> This is a webmaster/sysadmin error so we abort 500.
>
The folks I've talked to on the "don't turn HTTPExceptions into
responses by default" side of the debate argue that both use cases above
are poor coding practices because only "view code" (code that is
contacted only because a view was matched, as opposed to random things
that that view may call into) has enough information to be able to
return a sensible response. They further argue that since it's so easy
to "turn on" the feature, allowing folks that don't share their opinion
to do it, Pyramid shouldn't do it by default. Just FYI, that's the
other side of the argument.
IMO, all the other stuff (#2 thru 4) depends on this decision, and is
therefore detail-y ancillary stuff.
- C
Mike Orr
>> #1 is important so that you can cut through multiple function calls
> if
>> you discover an error condition.
>> Use case 1: a support method/function for several handlers
>> discovers a required query parameter is missing. The application's
>> forms and links would never produce such a request, so we presume the
>> request is illegitimate and abort 400.
>
> responses by default" side of the debate argue that both use cases above
> are poor coding practices because only "view code" (code that is
> contacted only because a view was matched, as opposed to random things
> that that view may call into) has enough information to be able to
> return a sensible response. They further argue that since it's so easy
> to "turn on" the feature, allowing folks that don't share their opinion
> to do it, Pyramid shouldn't do it by default. Just FYI, that's the
> other side of the argument.
OK, but there's such a thing as view support methods, code that's
common to several views so it doesn't have to be repeated in all of
them. That's the only place where I'd use this. For instance:
class MyHandler(object):
def my_view(self):
params = self.request.params
self._process_id(params.get("id"))
# Private methods
def _process_id(self, id_str):
if id_str is None or not id_str.isdigit():
abort(400, "query param 'id' missing")
id = str(id)
self.record = model.Something.get(id)
if self.record is None:
abort(404, "that Something does not exist")
--
Mike Orr <slugg...@gmail.com>
Wichert Akkerman
> OK, but there's such a thing as view support methods, code that's
> common to several views so it doesn't have to be repeated in all of
> them. That's the only place where I'd use this. For instance:
>
> class MyHandler(object):
> def my_view(self):
> params = self.request.params
> self._process_id(params.get("id"))
>
> # Private methods
> def _process_id(self, id_str):
> if id_str is None or not id_str.isdigit():
> abort(400, "query param 'id' missing")
> id = str(id)
> self.record = model.Something.get(id)
> if self.record is None:
> abort(404, "that Something does not exist")
Personally I find that this coding style makes it hard to read code: you
are basically rewriting a standard language feature (raising an
exception) in a way that makes it look like a normal function call.
Wichert.
--
Wichert Akkerman <wic...@wiggy.net> It is simple to make things.
http://www.wiggy.net/ It is hard to make things simple.
Mike Orr
> On 2011-6-2 19:20, Mike Orr wrote:
>>
>> OK, but there's such a thing as view support methods, code that's
>> common to several views so it doesn't have to be repeated in all of
>> them. That's the only place where I'd use this. For instance:
>>
>> class MyHandler(object):
>> def my_view(self):
>> params = self.request.params
>> self._process_id(params.get("id"))
>>
>> # Private methods
>> def _process_id(self, id_str):
>> if id_str is None or not id_str.isdigit():
>> abort(400, "query param 'id' missing")
>> id = str(id)
>> self.record = model.Something.get(id)
>> if self.record is None:
>> abort(404, "that Something does not exist")
>
> Personally I find that this coding style makes it hard to read code: you are
> basically rewriting a standard language feature (raising an exception) in a
> way that makes it look like a normal function call.
That's only in the difference between "abort()" and "raise abort()".
As I've said, I'm not opposed to requiring the latter, and I think it
might actually be a good thing.
--
Mike Orr <slugg...@gmail.com>
Wichert Akkerman
I have no objections to 'raise abort()'; that would make abort a simple
http exception factory.
Wichert.
Michael Merickel
Chris McDonough
The issue is with how Pyramid uses (or really more how it doesn't use
parts of) webob.Response.
Currently view callables that don't use a renderer are obligated to
return an object with this interface:
class IResponse(Interface):
status = Attribute('WSGI status code of response')
headerlist = Attribute('List of response headers')
app_iter = Attribute('Iterable representing the response body')
That is, the only restriction that Pyramid puts upon view callable code
is that it must return an object with those three attributes. Pyramid
doesn't care if that object is a webob.Response object. Indeed Pyramid
internally uses IResponse objects that implement this interface that do
not inherit from webob.Response (NotFound and Forbidden currently).
In the meantime, the conditional response code within WebOb is only
executed when a webob.Response object is treated as a WSGI application
(its __call__ is called with an "environ" and a "start_response").
Pyramid never uses a webob response object as a WSGI application,
however; it's __call__ is never called. This means that its
conditional response code is ignored. This is that code:
https://bitbucket.org/ianb/webob/src/411997824d3b/webob/response.py#cl-942 .
This isn't ideal.
- C