Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On 11 July 2013 20:50, Mark McLoughlin mar...@redhat.com wrote: That answer does begin with this, though: I almost always use hasattr: it's the correct choice for most cases. I have to disagree here, hasattr is basically a timebomb. Three-param getattr, or safe_hasattr are *much* better choices in every case I've seen so far. (safe_hasattr is in extras, or I believe there is a copy in oslo now). -Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Cloud Services ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On 07/11/2013 05:20 AM, Mark McLoughlin wrote:
On Wed, 2013-07-10 at 19:49 -0400, Monty Taylor wrote:
I'd like top-post and hijack this thread for another exception related
thing:
a) Anyone writing code such as:
try:
blah()
except SomeException:
raise SomeOtherExceptionLeavingOutStackContextFromSomeException
should be mocked ruthlessly.
Ok, mock me ruthlessly then.
Mock. Mock. Mock. Mock.
Part of designing any API is specifying what predictable exceptions it
will raise. For any predictable error condition, you don't want callers
to have to catch random exceptions from the underlying libraries you
might be calling into.
Absolutely. But you don't want to go so overboard that you remove the
ability for a developer to debug it. More importantly, INSIDE of server
code, we're not designing fun apis for the consumption of people we
don't know. There is clearly an API, but we are the consumers of our own
API.
Say if I was designing an image downloading API, I'd do something like
this:
https://gist.github.com/markmc/5973868
Assume there's a tonne more stuff that the API would do. You don't want
callers to have to catch socket.error exceptions and whatever other
exceptions might be thrown.
That works out as:
Traceback (most recent call last):
File t.py, line 20, in module
download_image('localhost', 3366, 'foobar')
File t.py, line 18, in download_image
raise ImageDownloadFailure(host, port, path, e.strerror)
__main__.ImageDownloadFailure: Failed to download foobar from
localhost:3366: Connection refused
Which is a pretty clear exception.
Right, but what if the message from the exception does not give you
enough information to walk down the stack and see it...
Also, I have more than once seen:
class MyIOError(Exception):
pass
try:
s = socket.create_connection((host, port))
except socket.error:
raise MyIOError(something went wrong!)
Which is an extreme example of where my rage comes from, but not a made
up example. I have, actually, seen code exactly like that - in Nova.
BTW - I'd have download_image return None if it can't download the
image, and I'd have it either deal with the socket.error or not locally
at the source. But now we're nitpicking.
But I think what you're saying is missing is the stack trace from the
underlying exception.
YES - and I'll let David's response respond to the details of the rest...
But essentially, what I was really mocking earlier is throwing a new
exception in such a way that you lose the exception propagation path. So
if you throw an exception inside of an except block, you should be sure
that you're passing on all of the info, because if a developer gets an
exception, it's quite likely that they want to know how to fix it. :)
As I understood it, Python doesn't have a way of chaining exceptions
like this but e.g. Java does. A little bit more poking right now shows
up this:
http://www.python.org/dev/peps/pep-3134/
i.e. we can't do the right thing until Python 3, where we'd do:
def download_image(host, port, path):
try:
s = socket.create_connection((host, port))
except socket.error as e:
raise ImageDownloadFailure(host, port, path, e.strerror) from e
This will certainly be cleaner to write and read.
I haven't read the PEP in detail yet, though.
Cheers,
Mark.
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On 07/11/2013 05:43 AM, Thomas Hervé wrote: On Thu, Jul 11, 2013 at 1:49 AM, Monty Taylor mord...@inaugust.com mailto:mord...@inaugust.com wrote: I'd like top-post and hijack this thread for another exception related thing: a) Anyone writing code such as: try: blah() except SomeException: raise SomeOtherExceptionLeavingOutStackContextFromSomeException should be mocked ruthlessly. i don't think mocking is a good way to convey your point. Losing tracebacks is not great, but having an API raising random exceptions is probably worse. Can you develop? Mocking is bad at many things, and email is a bad way to express generalizations through exaggeration. I would almost certainly not actually mock anyone. I doubt very seriously that any of us here working on OpenStack lack the ability to develop. In response to your question though- I find that when the process of debugging an error involves editing an installed library to remove an improperly done exception translation so that I can actually see the traceback and find the error, then someone has made the wrong choice in their implementation of the library. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On 07/11/2013 05:43 AM, Thomas Hervé wrote: On Thu, Jul 11, 2013 at 1:49 AM, Monty Taylor mord...@inaugust.com mailto:mord...@inaugust.com wrote: I'd like top-post and hijack this thread for another exception related thing: a) Anyone writing code such as: try: blah() except SomeException: raise SomeOtherExceptionLeavingOutStackContextFromSomeException should be mocked ruthlessly. i don't think mocking is a good way to convey your point. Losing tracebacks is not great, but having an API raising random exceptions is probably worse. Can you develop? I have learned that I may have mis-read your last three words due to translation problems. You were not asking if I had the ability to write code, rather you were asking if I could elaborate. I will consider that I have learned something new today! Regards, -- Thomas ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On 07/11/2013 06:22 AM, Sean Dague wrote: On 07/11/2013 05:43 AM, Thomas Hervé wrote: On Thu, Jul 11, 2013 at 1:49 AM, Monty Taylor mord...@inaugust.com mailto:mord...@inaugust.com wrote: I'd like top-post and hijack this thread for another exception related thing: a) Anyone writing code such as: try: blah() except SomeException: raise SomeOtherExceptionLeavingOutStackContextFromSomeException should be mocked ruthlessly. i don't think mocking is a good way to convey your point. Losing tracebacks is not great, but having an API raising random exceptions is probably worse. Can you develop? We have defined APIs. We have server projects that call other server projects through python-fooclients. We have python-fooclients that generate exceptions and stack traces on any non 20X return. So we have a general exception translation issue. Because unless we want nova-api returns to be completely fluid based on what keystone/neutron/glance/cinder clients decided their exceptions are this week, you have to translate. And the stack trace from a 404 isn't something that we really care about from the client, we care that it happened, because that affects nova logic, but it's an expected exception. It's an expected exception until it's not. And where would we even put the stack trace? take it to the user on their API call? Oh gosh no! I'm CERTAINLY not suggesting that we passthrough translated exceptions to our REST API consumers. I'm talking about actual exceptions in server code which generate actual error logging because they are actual errors but where you cannot find what the error is because of masking. fill up the logs with stack traces for normal events (thus hiding real errors)? (we actually have some blueprints openned now to remove these because a *normal* passing tempest run generates 30+ stack traces in nova logs, which aren't actually internal errors.) Yes. This is actually much worse, and I fully support you in all of these efforts! The bulk of these cases I've seen in Nova are exactly of that nature. We actually have a pretty standard pattern of 3 levels of exception translations for user API calls. Underlying client exception (be it DB / python-fooclient) - Nova internal exception - Webobj exception to create our 40x / 50x returns. I believe striping the underlying exception in the webobj translation is perfectly reasonable. The alternative would be madness. I honestly can't find a great example of this in our code this second (I think concrete examples are great) but I know that I've hit it enough to lodge an annoyance flag - so let me pull an example from the insides of our friend d2to1: try: attrs = cfg_to_args(path) except: e = sys.exc_info()[1] raise DistutilsSetupError( 'Error parsing %s: %s: %s' % (path, e.__class__.__name__, e.args[0])) This is an attempt to give the user a meaningful error in the case that they have a problem. So - first thing is, there's a bug, becuase e.args[0] is the error code, so you get: error in setup command: Error parsing setup.cfg: IOError: 2 Which is TOTALLY meaningless. If you change the string line to: +'Error parsing %s: %s: %s' % (path, e.__class__.__name__, e)) You at least get: error in setup command: Error parsing setup.cfg: IOError: No such file or directory: 'README' Which is much more helpful. However, that's a simple error, if you have a more complex error, such as your pbr hook threw a traceback while loading, it's a bit bonghits and to track it down you may have to expend some effort. BUT - for the general case, you have a typo in your setup.cfg file, IOError: No such file or directory: 'README' is probably sufficient and you don't probably need the traceback. That said - when I'm having problems using this code, I tend to just edit that file, remove that exception wrapper altogether, and let the exceptions fly- and can usually find the problem in about a second. Maybe I should add a debug flag which skips the wrapping... ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Fri, Jul 12, 2013 at 5:33 PM, Monty Taylor mord...@inaugust.com wrote: On 07/11/2013 05:43 AM, Thomas Hervé wrote: On Thu, Jul 11, 2013 at 1:49 AM, Monty Taylor mord...@inaugust.com mailto:mord...@inaugust.com wrote: I'd like top-post and hijack this thread for another exception related thing: a) Anyone writing code such as: try: blah() except SomeException: raise SomeOtherExceptionLeavingOutStackContextFromSomeException should be mocked ruthlessly. i don't think mocking is a good way to convey your point. Losing tracebacks is not great, but having an API raising random exceptions is probably worse. Can you develop? I have learned that I may have mis-read your last three words due to translation problems. You were not asking if I had the ability to write code, rather you were asking if I could elaborate. Ah thanks, and sorry for the frenchism. I think I understand your point now, which is not so much about tracebacks but about context in the wide sense, ie enough information to understand what's going on. I've found that we're not necessarily doing a great job at testing the error messages: it's nice to know that FooError has been raised, but if the message is 'Error' it leads to what you're describing, where you need to look at the code and potentially change it to debug it. I believe more consistent tests may help that a bit. Cheers, -- Thomas ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Fri, Jul 12, 2013 at 10:09 AM, Monty Taylor mord...@inaugust.com wrote:
On 07/11/2013 05:20 AM, Mark McLoughlin wrote:
On Wed, 2013-07-10 at 19:49 -0400, Monty Taylor wrote:
I'd like top-post and hijack this thread for another exception related
thing:
a) Anyone writing code such as:
try:
blah()
except SomeException:
raise SomeOtherExceptionLeavingOutStackContextFromSomeException
should be mocked ruthlessly.
Ok, mock me ruthlessly then.
Mock. Mock. Mock. Mock.
Part of designing any API is specifying what predictable exceptions it
will raise. For any predictable error condition, you don't want callers
to have to catch random exceptions from the underlying libraries you
might be calling into.
Absolutely. But you don't want to go so overboard that you remove the
ability for a developer to debug it. More importantly, INSIDE of server
code, we're not designing fun apis for the consumption of people we
don't know. There is clearly an API, but we are the consumers of our own
API.
Lies! Write everything to be intuitive for new contributors or we won't
have any :(
Say if I was designing an image downloading API, I'd do something like
this:
https://gist.github.com/markmc/5973868
Assume there's a tonne more stuff that the API would do. You don't want
callers to have to catch socket.error exceptions and whatever other
exceptions might be thrown.
That works out as:
Traceback (most recent call last):
File t.py, line 20, in module
download_image('localhost', 3366, 'foobar')
File t.py, line 18, in download_image
raise ImageDownloadFailure(host, port, path, e.strerror)
__main__.ImageDownloadFailure: Failed to download foobar from
localhost:3366: Connection refused
Which is a pretty clear exception.
Right, but what if the message from the exception does not give you
enough information to walk down the stack and see it...
Also, I have more than once seen:
class MyIOError(Exception):
pass
try:
s = socket.create_connection((host, port))
except socket.error:
raise MyIOError(something went wrong!)
Which is an extreme example of where my rage comes from, but not a made
up example. I have, actually, seen code exactly like that - in Nova.
BTW - I'd have download_image return None if it can't download the
image, and I'd have it either deal with the socket.error or not locally
at the source. But now we're nitpicking.
But I think what you're saying is missing is the stack trace from the
underlying exception.
YES - and I'll let David's response respond to the details of the rest...
But essentially, what I was really mocking earlier is throwing a new
exception in such a way that you lose the exception propagation path. So
if you throw an exception inside of an except block, you should be sure
that you're passing on all of the info, because if a developer gets an
exception, it's quite likely that they want to know how to fix it. :)
As I understood it, Python doesn't have a way of chaining exceptions
like this but e.g. Java does. A little bit more poking right now shows
up this:
http://www.python.org/dev/peps/pep-3134/
i.e. we can't do the right thing until Python 3, where we'd do:
def download_image(host, port, path):
try:
s = socket.create_connection((host, port))
except socket.error as e:
raise ImageDownloadFailure(host, port, path, e.strerror) from e
This will certainly be cleaner to write and read.
I haven't read the PEP in detail yet, though.
Cheers,
Mark.
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
--
-Dolph
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
Hi folks
Monty
Thank you for your adding good topic for this.
I agree, hiding true cause by exception get hard to debug
For, example, Sqlalchemy gives me this error for general sql errors..
exceptions must be old-style classes or derived from BaseException, not str
(It should be hidden from REST API users though)
Also, I agree with you about log policies.
sometimes 404 get warn log..
IMO, The log more than warn level should help Operators.
also If the exception is truly, the exceptional case which needs help
of operators.
It should be logged as error level.
Thanks
Nachi
2013/7/12 Dolph Mathews dolph.math...@gmail.com:
On Fri, Jul 12, 2013 at 10:09 AM, Monty Taylor mord...@inaugust.com wrote:
On 07/11/2013 05:20 AM, Mark McLoughlin wrote:
On Wed, 2013-07-10 at 19:49 -0400, Monty Taylor wrote:
I'd like top-post and hijack this thread for another exception related
thing:
a) Anyone writing code such as:
try:
blah()
except SomeException:
raise SomeOtherExceptionLeavingOutStackContextFromSomeException
should be mocked ruthlessly.
Ok, mock me ruthlessly then.
Mock. Mock. Mock. Mock.
Part of designing any API is specifying what predictable exceptions it
will raise. For any predictable error condition, you don't want callers
to have to catch random exceptions from the underlying libraries you
might be calling into.
Absolutely. But you don't want to go so overboard that you remove the
ability for a developer to debug it. More importantly, INSIDE of server
code, we're not designing fun apis for the consumption of people we
don't know. There is clearly an API, but we are the consumers of our own
API.
Lies! Write everything to be intuitive for new contributors or we won't have
any :(
Say if I was designing an image downloading API, I'd do something like
this:
https://gist.github.com/markmc/5973868
Assume there's a tonne more stuff that the API would do. You don't want
callers to have to catch socket.error exceptions and whatever other
exceptions might be thrown.
That works out as:
Traceback (most recent call last):
File t.py, line 20, in module
download_image('localhost', 3366, 'foobar')
File t.py, line 18, in download_image
raise ImageDownloadFailure(host, port, path, e.strerror)
__main__.ImageDownloadFailure: Failed to download foobar from
localhost:3366: Connection refused
Which is a pretty clear exception.
Right, but what if the message from the exception does not give you
enough information to walk down the stack and see it...
Also, I have more than once seen:
class MyIOError(Exception):
pass
try:
s = socket.create_connection((host, port))
except socket.error:
raise MyIOError(something went wrong!)
Which is an extreme example of where my rage comes from, but not a made
up example. I have, actually, seen code exactly like that - in Nova.
BTW - I'd have download_image return None if it can't download the
image, and I'd have it either deal with the socket.error or not locally
at the source. But now we're nitpicking.
But I think what you're saying is missing is the stack trace from the
underlying exception.
YES - and I'll let David's response respond to the details of the rest...
But essentially, what I was really mocking earlier is throwing a new
exception in such a way that you lose the exception propagation path. So
if you throw an exception inside of an except block, you should be sure
that you're passing on all of the info, because if a developer gets an
exception, it's quite likely that they want to know how to fix it. :)
As I understood it, Python doesn't have a way of chaining exceptions
like this but e.g. Java does. A little bit more poking right now shows
up this:
http://www.python.org/dev/peps/pep-3134/
i.e. we can't do the right thing until Python 3, where we'd do:
def download_image(host, port, path):
try:
s = socket.create_connection((host, port))
except socket.error as e:
raise ImageDownloadFailure(host, port, path, e.strerror) from e
This will certainly be cleaner to write and read.
I haven't read the PEP in detail yet, though.
Cheers,
Mark.
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
--
-Dolph
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
Somewhat offtopic, but others might find it interesting. On another project
I used a different technique for exceptions, where the original exception
is enhanced with new fields as it propagates up the stack to where it's
handled. In this way all the information needed to handle the exception is
available (even if it's just to log it).
Often there's some information that would be useful for the exception
handler that isn't available at the place where the exception is raised.
The classic example is an error writing to a file where you'd like to know
the filename but all you've got is the file descriptor. An example from an
OpenStack REST API might be that you get an exception and you'd like to log
a message that includes the resource path, but the resource path isn't
known at this point.
So rather than logging the exception when it happens, enhance the
exception, re-raise it, and only once it's got all the information where
you can print out a useful log message you log it.
Here's a short example to illustrate. An exception is raised by f1(), but
at this point we don't know the status code that the server should respond
with or the request path which would be useful in a log message. These bits
of information are added as the exception propagates and then where it's
caught we've got all the information for a useful log message.
def f1():
raise Exception('something')
def f2():
try:
f1()
except Exception as e:
e.status_code = 403
raise
def do_tokens():
try:
f2()
except Exception as e:
e.req_url = '/v3/tokens'
raise
status_code = 200
try:
do_tokens()
except Exception as e:
status_code = getattr(e, 'status_code', 500)
req_url = getattr(e, 'req_url', 'unknown')
if status_code != 200:
print 'Got %s from %s' % (status_code, req_url)
# build an error document for the response.
On Fri, Jul 12, 2013 at 12:25 PM, Nachi Ueno na...@ntti3.com wrote:
Hi folks
Monty
Thank you for your adding good topic for this.
I agree, hiding true cause by exception get hard to debug
For, example, Sqlalchemy gives me this error for general sql errors..
exceptions must be old-style classes or derived from BaseException, not
str
(It should be hidden from REST API users though)
Also, I agree with you about log policies.
sometimes 404 get warn log..
IMO, The log more than warn level should help Operators.
also If the exception is truly, the exceptional case which needs help
of operators.
It should be logged as error level.
Thanks
Nachi
2013/7/12 Dolph Mathews dolph.math...@gmail.com:
On Fri, Jul 12, 2013 at 10:09 AM, Monty Taylor mord...@inaugust.com
wrote:
On 07/11/2013 05:20 AM, Mark McLoughlin wrote:
On Wed, 2013-07-10 at 19:49 -0400, Monty Taylor wrote:
I'd like top-post and hijack this thread for another exception
related
thing:
a) Anyone writing code such as:
try:
blah()
except SomeException:
raise SomeOtherExceptionLeavingOutStackContextFromSomeException
should be mocked ruthlessly.
Ok, mock me ruthlessly then.
Mock. Mock. Mock. Mock.
Part of designing any API is specifying what predictable exceptions it
will raise. For any predictable error condition, you don't want
callers
to have to catch random exceptions from the underlying libraries you
might be calling into.
Absolutely. But you don't want to go so overboard that you remove the
ability for a developer to debug it. More importantly, INSIDE of server
code, we're not designing fun apis for the consumption of people we
don't know. There is clearly an API, but we are the consumers of our own
API.
Lies! Write everything to be intuitive for new contributors or we won't
have
any :(
Say if I was designing an image downloading API, I'd do something like
this:
https://gist.github.com/markmc/5973868
Assume there's a tonne more stuff that the API would do. You don't
want
callers to have to catch socket.error exceptions and whatever other
exceptions might be thrown.
That works out as:
Traceback (most recent call last):
File t.py, line 20, in module
download_image('localhost', 3366, 'foobar')
File t.py, line 18, in download_image
raise ImageDownloadFailure(host, port, path, e.strerror)
__main__.ImageDownloadFailure: Failed to download foobar from
localhost:3366: Connection refused
Which is a pretty clear exception.
Right, but what if the message from the exception does not give you
enough information to walk down the stack and see it...
Also, I have more than once seen:
class MyIOError(Exception):
pass
try:
s = socket.create_connection((host, port))
except socket.error:
raise MyIOError(something went wrong!)
Which is an extreme example of where my rage comes from, but not a made
up example. I have, actually, seen code exactly like that - in Nova.
BTW - I'd have
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
I'm curious if folks know about this?
import sys
def foo():
raise Exception('Foo Exception')
def bar():
try:
foo()
except Exception:
exc_info = sys.exc_info()
raise Exception('Bar Exception'), None, exc_info[2]
bar()
Traceback (most recent call last):
File test.py, line 15, in module
bar()
File test.py, line 9, in bar
foo()
File test.py, line 4, in foo
raise Exception('Foo Exception')
Exception: Bar Exception
On Fri, Jul 12, 2013 at 3:53 PM, Doug Hellmann
doug.hellm...@dreamhost.comwrote:
On Fri, Jul 12, 2013 at 2:40 PM, Brant Knudson b...@acm.org wrote:
Somewhat offtopic, but others might find it interesting. On another
project I used a different technique for exceptions, where the original
exception is enhanced with new fields as it propagates up the stack to
where it's handled. In this way all the information needed to handle the
exception is available (even if it's just to log it).
Often there's some information that would be useful for the exception
handler that isn't available at the place where the exception is raised.
The classic example is an error writing to a file where you'd like to know
the filename but all you've got is the file descriptor. An example from an
OpenStack REST API might be that you get an exception and you'd like to log
a message that includes the resource path, but the resource path isn't
known at this point.
So rather than logging the exception when it happens, enhance the
exception, re-raise it, and only once it's got all the information where
you can print out a useful log message you log it.
Here's a short example to illustrate. An exception is raised by f1(), but
at this point we don't know the status code that the server should respond
with or the request path which would be useful in a log message. These bits
of information are added as the exception propagates and then where it's
caught we've got all the information for a useful log message.
def f1():
raise Exception('something')
def f2():
try:
f1()
except Exception as e:
e.status_code = 403
raise
def do_tokens():
try:
f2()
except Exception as e:
e.req_url = '/v3/tokens'
raise
status_code = 200
try:
do_tokens()
except Exception as e:
status_code = getattr(e, 'status_code', 500)
req_url = getattr(e, 'req_url', 'unknown')
if status_code != 200:
print 'Got %s from %s' % (status_code, req_url)
# build an error document for the response.
One problem with this approach is it spreads knowledge of the error
construction up and down the stack through different layers of the
application, and that brings with it assumptions about the implementation
at the different layers. For example, should the application code above
know that do_tokens() is making web requests to URLs? Why does it need that
information?
SRP-ly,
Doug
On Fri, Jul 12, 2013 at 12:25 PM, Nachi Ueno na...@ntti3.com wrote:
Hi folks
Monty
Thank you for your adding good topic for this.
I agree, hiding true cause by exception get hard to debug
For, example, Sqlalchemy gives me this error for general sql errors..
exceptions must be old-style classes or derived from BaseException, not
str
(It should be hidden from REST API users though)
Also, I agree with you about log policies.
sometimes 404 get warn log..
IMO, The log more than warn level should help Operators.
also If the exception is truly, the exceptional case which needs help
of operators.
It should be logged as error level.
Thanks
Nachi
2013/7/12 Dolph Mathews dolph.math...@gmail.com:
On Fri, Jul 12, 2013 at 10:09 AM, Monty Taylor mord...@inaugust.com
wrote:
On 07/11/2013 05:20 AM, Mark McLoughlin wrote:
On Wed, 2013-07-10 at 19:49 -0400, Monty Taylor wrote:
I'd like top-post and hijack this thread for another exception
related
thing:
a) Anyone writing code such as:
try:
blah()
except SomeException:
raise SomeOtherExceptionLeavingOutStackContextFromSomeException
should be mocked ruthlessly.
Ok, mock me ruthlessly then.
Mock. Mock. Mock. Mock.
Part of designing any API is specifying what predictable exceptions
it
will raise. For any predictable error condition, you don't want
callers
to have to catch random exceptions from the underlying libraries you
might be calling into.
Absolutely. But you don't want to go so overboard that you remove the
ability for a developer to debug it. More importantly, INSIDE of
server
code, we're not designing fun apis for the consumption of people we
don't know. There is clearly an API, but we are the consumers of our
own
API.
Lies! Write everything to be intuitive for new contributors or we
won't have
any :(
Say if I was designing an image downloading API, I'd do something
like
this:
https://gist.github.com/markmc/5973868
Assume there's a
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Wed, 2013-07-10 at 21:14 +0200, Thomas Hervé wrote: On Wed, Jul 10, 2013 at 8:32 PM, Mark McLoughlin mar...@redhat.com wrote: On Wed, 2013-07-10 at 11:01 -0700, Nachi Ueno wrote: Personally, I prefer not to use exception for such cases. The key here is personally. I don't think we have to agree on all style issues. When it results in a patch submitter getting a -1 from one person for choosing EAFP and a -1 from another person for choosing LBYL, then yes ... actually we do need to agree. My instinct is the same, but EAFP does seem to be the python way. There are times I can tolerate the EAFP approach but, even then, I generally think LBYL is cleaner. I can live with something like this: try: return obj.foo except AttributeError: pass but this is obviously broken: try: return self.do_something(obj.foo) except AttributeError: pass since AttributeError will mask a typo with the do_something() call or an AttributeError raised from inside do_something() But I fail to see what's wrong with this: if hasattr(obj, 'foo'): return obj.foo hasattr is a bit dangerous as it catches more exceptions than it needs too. See for example http://stackoverflow.com/questions/903130/hasattr-vs-try-except-block-to-deal-with-non-existent-attributes/16186050#16186050 for an explanation. That answer does begin with this, though: I almost always use hasattr: it's the correct choice for most cases. and, frankly, a __getattr__() method that returns ValueError is broken. i.e. the conclusion would be that we should only avoid hasattr() in some very limited cases where the underlying __getattr__() does weird things or where using it can result in a race condition. Cheers, Mark. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Wed, 2013-07-10 at 19:49 -0400, Monty Taylor wrote:
I'd like top-post and hijack this thread for another exception related
thing:
a) Anyone writing code such as:
try:
blah()
except SomeException:
raise SomeOtherExceptionLeavingOutStackContextFromSomeException
should be mocked ruthlessly.
Ok, mock me ruthlessly then.
Part of designing any API is specifying what predictable exceptions it
will raise. For any predictable error condition, you don't want callers
to have to catch random exceptions from the underlying libraries you
might be calling into.
Say if I was designing an image downloading API, I'd do something like
this:
https://gist.github.com/markmc/5973868
Assume there's a tonne more stuff that the API would do. You don't want
callers to have to catch socket.error exceptions and whatever other
exceptions might be thrown.
That works out as:
Traceback (most recent call last):
File t.py, line 20, in module
download_image('localhost', 3366, 'foobar')
File t.py, line 18, in download_image
raise ImageDownloadFailure(host, port, path, e.strerror)
__main__.ImageDownloadFailure: Failed to download foobar from localhost:3366:
Connection refused
Which is a pretty clear exception.
But I think what you're saying is missing is the stack trace from the
underlying exception.
As I understood it, Python doesn't have a way of chaining exceptions
like this but e.g. Java does. A little bit more poking right now shows
up this:
http://www.python.org/dev/peps/pep-3134/
i.e. we can't do the right thing until Python 3, where we'd do:
def download_image(host, port, path):
try:
s = socket.create_connection((host, port))
except socket.error as e:
raise ImageDownloadFailure(host, port, path, e.strerror) from e
I haven't read the PEP in detail yet, though.
Cheers,
Mark.
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Thu, Jul 11, 2013 at 5:20 AM, Mark McLoughlin mar...@redhat.com wrote: But I think what you're saying is missing is the stack trace from the underlying exception. As I understood it, Python doesn't have a way of chaining exceptions like this but e.g. Java does. A little bit more poking right now shows up this: http://www.python.org/dev/peps/pep-3134/ i.e. we can't do the right thing until Python 3, where we'd do: def download_image(host, port, path): try: s = socket.create_connection((host, port)) except socket.error as e: raise ImageDownloadFailure(host, port, path, e.strerror) from e I haven't read the PEP in detail yet, though. You can actually do this in Python 2 and keep the original context: def download_image(host, port, path): try: s = socket.create_connection((host, port)) except socket.error as e: raise ImageDownloadFailure, e, sys.exc_info()[-1] This will keep the original message and stack trace, but change the type. You can also change the message if you want my mucking with e's message. I've done that to add a string like (socket.error) at the end of the exception message so I could see the original type. If you really, really wanted to use a bare except you could also do something like: try: do_something_that_raises_an_exception() except: exc_value, exc_tb = sys.exc_info()[1:] raise MyException, exc_value, exc_tb -- David blog: http://www.traceback.org twitter: http://twitter.com/dstanek www: http://dstanek.com ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [Review] Use of exception for non-exceptional cases
HI folks I would like to ask the review criteria in the community. Should we use exception for non-exceptional cases when we can use parameter checking? Example1: Default value for array index try: value = list[5] except IndexError: value = 'default_value' This can be also written as, list_a[3] if len(list_a) 3 else 'default_value' ask for forgiveness, not permission is one of way in python, however, on the other hand, google python code style guide says, - Minimize the amount of code in a try/except block. The larger the body of the try, the more likely that an exception will be raised by a line of code that you didn't expect to raise an exception. In those cases, the try/except block hides a real error. --- http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Exceptions Personally, I prefer not to use exception for such cases. Best Nachi ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Wed, Jul 10, 2013 at 1:01 PM, Nachi Ueno na...@ntti3.com wrote: HI folks I would like to ask the review criteria in the community. Should we use exception for non-exceptional cases when we can use parameter checking? Example1: Default value for array index try: value = list[5] except IndexError: value = 'default_value' I can't get past this specific example... how often do you find yourself needing to do this, exactly? Generally when you use a list you either FIFO / LIFO or iterate through the whole thing in some fashion. I'd be tempted to write it as dict(enumerate(my_list)).get(3, 'default_value') just because you're treating it like a mapping anyway. This can be also written as, list_a[3] if len(list_a) 3 else 'default_value' ask for forgiveness, not permission is one of way in python, however, on the other hand, google python code style guide says, - Minimize the amount of code in a try/except block. The larger the body of the try, the more likely that an exception will be raised by a line of code that you didn't expect to raise an exception. In those cases, the try/except block hides a real error. --- http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Exceptions +1 for this, but it's not really intended to provide an answer your question of approach. Personally, I prefer not to use exception for such cases. Best Nachi ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- -Dolph ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Wed, 2013-07-10 at 11:01 -0700, Nachi Ueno wrote: HI folks I would like to ask the review criteria in the community. Should we use exception for non-exceptional cases when we can use parameter checking? Example1: Default value for array index try: value = list[5] except IndexError: value = 'default_value' This can be also written as, list_a[3] if len(list_a) 3 else 'default_value' ask for forgiveness, not permission is one of way in python, however, on the other hand, google python code style guide says, - Minimize the amount of code in a try/except block. The larger the body of the try, the more likely that an exception will be raised by a line of code that you didn't expect to raise an exception. In those cases, the try/except block hides a real error. --- http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Exceptions I don't think this statement contradicts the intent of EAFP. Personally, I prefer not to use exception for such cases. My instinct is the same, but EAFP does seem to be the python way. There are times I can tolerate the EAFP approach but, even then, I generally think LBYL is cleaner. I can live with something like this: try: return obj.foo except AttributeError: pass but this is obviously broken: try: return self.do_something(obj.foo) except AttributeError: pass since AttributeError will mask a typo with the do_something() call or an AttributeError raised from inside do_something() But I fail to see what's wrong with this: if hasattr(obj, 'foo'): return obj.foo Cheers, Mark. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
Hi Mark Thank you for your answering I don't think this statement contradicts the intent of EAFP. I got it :) Personally, I prefer not to use exception for such cases. My instinct is the same, but EAFP does seem to be the python way. There are times I can tolerate the EAFP approach but, even then, I generally think LBYL is cleaner. ok, so i'm worrying about the case one reviewer says to use LBYL, and the other mentioning EAFP. so it is great if we could some some criteria for this. I can live with something like this: try: return obj.foo except AttributeError: pass but this is obviously broken: try: return self.do_something(obj.foo) except AttributeError: pass since AttributeError will mask a typo with the do_something() call or an AttributeError raised from inside do_something() But I fail to see what's wrong with this: if hasattr(obj, 'foo'): return obj.foo Cheers, Mark. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Wed, Jul 10, 2013 at 8:32 PM, Mark McLoughlin mar...@redhat.com wrote: On Wed, 2013-07-10 at 11:01 -0700, Nachi Ueno wrote: Personally, I prefer not to use exception for such cases. The key here is personally. I don't think we have to agree on all style issues. My instinct is the same, but EAFP does seem to be the python way. There are times I can tolerate the EAFP approach but, even then, I generally think LBYL is cleaner. I can live with something like this: try: return obj.foo except AttributeError: pass but this is obviously broken: try: return self.do_something(obj.foo) except AttributeError: pass since AttributeError will mask a typo with the do_something() call or an AttributeError raised from inside do_something() But I fail to see what's wrong with this: if hasattr(obj, 'foo'): return obj.foo hasattr is a bit dangerous as it catches more exceptions than it needs too. See for example http://stackoverflow.com/questions/903130/hasattr-vs-try-except-block-to-deal-with-non-existent-attributes/16186050#16186050for an explanation. -- Thomas ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
Hi Thomas Thank you for your reply. The key here is personally. I don't think we have to agree on all style issues. personally is why I'm asking it for communities. IMO, we should agree on the style issue as much as we can. (Eg pep8, flake8) for more consistant review. However, I also agree it is hard to agree on all style issues, and sometimes it is case by case. My instinct is the same, but EAFP does seem to be the python way. There are times I can tolerate the EAFP approach but, even then, I generally think LBYL is cleaner. I can live with something like this: try: return obj.foo except AttributeError: pass but this is obviously broken: try: return self.do_something(obj.foo) except AttributeError: pass since AttributeError will mask a typo with the do_something() call or an AttributeError raised from inside do_something() But I fail to see what's wrong with this: if hasattr(obj, 'foo'): return obj.foo hasattr is a bit dangerous as it catches more exceptions than it needs too. See for example http://stackoverflow.com/questions/903130/hasattr-vs-try-except-block-to-deal-with-non-existent-attributes/16186050#16186050 for an explanation. Thank you for sharing this. I'll check the obj is overwriting __getattr__ or not on the review. # BTW, using __getattr__ should be also minimized. Thanks Nachi. -- Thomas ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On 07/10/2013 02:01 PM, Nachi Ueno wrote: HI folks I would like to ask the review criteria in the community. Should we use exception for non-exceptional cases when we can use parameter checking? Example1: Default value for array index try: value = list[5] except IndexError: value = 'default_value' This can be also written as, list_a[3] if len(list_a) 3 else 'default_value' Both of these are fine. Neither deserves to be banned. But LBYL is often naive in the face of concurrency. Just because something was true a microsecond ago doesn't mean it's still true. Exceptions are often more robust. -- David Ripton Red Hat drip...@redhat.com ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Wed, Jul 10, 2013 at 3:57 PM, David Ripton drip...@redhat.com wrote:
On 07/10/2013 02:01 PM, Nachi Ueno wrote:
HI folks
I would like to ask the review criteria in the community.
Should we use exception for non-exceptional cases when we can use
parameter checking?
Example1: Default value for array index
try:
value = list[5]
except IndexError:
value = 'default_value'
This can be also written as,
list_a[3] if len(list_a) 3 else 'default_value'
Both of these are fine. Neither deserves to be banned.
But LBYL is often naive in the face of concurrency. Just because
something was true a microsecond ago doesn't mean it's still true.
Exceptions are often more robust.
getattr() takes a default and, as it is implemented in C, is thread-safe.
So:
value = getattr(my_obj, 'might_not_be_there', 'default')
Of course, it's probably better to make sure you've always got the same
type of object in the first place but sometimes the attributes change
across versions of libraries.
For accessing elements of a sequence that may be too short,
itertools.chain() and itertools.islice() are useful.
import itertools
vals1 = ['a', 'b']
a, b, c = itertools.islice(itertools.chain(vals1, ['c']), 3)
a, b, c
('a', 'b', 'c')
vals2 = ['a', 'b', 'd']
a, b, c = itertools.islice(itertools.chain(vals2, ['c']), 3)
a, b, c
('a', 'b', 'd')
Doug
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Review] Use of exception for non-exceptional cases
On Wed, Jul 10, 2013 at 3:30 PM, Doug Hellmann
doug.hellm...@dreamhost.comwrote:
On Wed, Jul 10, 2013 at 3:57 PM, David Ripton drip...@redhat.com wrote:
On 07/10/2013 02:01 PM, Nachi Ueno wrote:
HI folks
I would like to ask the review criteria in the community.
Should we use exception for non-exceptional cases when we can use
parameter checking?
Example1: Default value for array index
try:
value = list[5]
except IndexError:
value = 'default_value'
This can be also written as,
list_a[3] if len(list_a) 3 else 'default_value'
Both of these are fine. Neither deserves to be banned.
But LBYL is often naive in the face of concurrency. Just because
something was true a microsecond ago doesn't mean it's still true.
Exceptions are often more robust.
getattr() takes a default and, as it is implemented in C, is thread-safe.
So:
value = getattr(my_obj, 'might_not_be_there', 'default')
Of course, it's probably better to make sure you've always got the same
type of object in the first place but sometimes the attributes change
across versions of libraries.
For accessing elements of a sequence that may be too short,
itertools.chain() and itertools.islice() are useful.
import itertools
vals1 = ['a', 'b']
a, b, c = itertools.islice(itertools.chain(vals1, ['c']), 3)
a, b, c
('a', 'b', 'c')
vals2 = ['a', 'b', 'd']
a, b, c = itertools.islice(itertools.chain(vals2, ['c']), 3)
a, b, c
('a', 'b', 'd')
++ every time I look at itertools it's doing something clever
Doug
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
--
-Dolph
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
