Re: ImportError catching in urlresolvers.py
On Thu, Jun 16, 2011 at 3:09 PM, Bas Peschier wrote: > For reference, I added a patch to https://code.djangoproject.com/ticket/10802 > which takes Russell's approach. Looks good to me! I've just marked to RFC. Thanks for the patch Bas! Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: ImportError catching in urlresolvers.py
For reference, I added a patch to https://code.djangoproject.com/ticket/10802 which takes Russell's approach. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: ImportError catching in urlresolvers.py
On 14.06.2011 22:19, Michael Blume wrote:
> In RegexURLPattern._get_callback, we attempt to fetch the callable named
> by the URL pattern, and catch a possible ImportError if this fails. If
> so, we raise ViewDoesNotExist.
>
> try:
> self._callback = get_callable(self._callback_str)
> except ImportError, e:
> mod_name, _ = get_mod_func(self._callback_str)
> raise ViewDoesNotExist("Could not import %s. Error was: %s"
> % (mod_name, str(e)))
I uncomment the try...except part in my local django version (and the
AttributeError). I want to see the original
exception. It would be nice to have a better solution in django. Maybe a
settings variable CATCH_EXCEPTIONS?
Thomas
--
Thomas Guettler, http://www.thomas-guettler.de/
E-Mail: guettli (*) thomas-guettler + de
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Re: ImportError catching in urlresolvers.py
On 15 jun, 07:36, Russell Keith-Magee wrote: > In this case, the underlying problem is that ImportError is actually > catching 2 different errors: > 1) The view specified doesn't exist > 2) The module containing the view contains an error. Agreed, in this case this information is lost in get_callable, where the code does know the difference between 1) and 2), but the exception raised does not convey that. It might be a better idea to move the ViewDoesNotExist into the get_callable function, which get triggered with an AttributeException or an ImportError when the owning module can be imported. All other conditions are not caught. Bas -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: ImportError catching in urlresolvers.py
On Wed, Jun 15, 2011 at 1:31 PM, burc...@gmail.com wrote:
> Hi Russell,
>
> and what do you say about showing call stack properly?
>
> The problem is not ViewDoesNotExist itself, but throwing away useful
> traceback.
>
> If we do instead:
> import sys
> try:
> self._callback = get_callable(self._callback_str)
> except ImportError, e:
> mod_name, _ = get_mod_func(self._callback_str)
> raise ViewDoesNotExist("Could not import %s. Error was: %s" %
> (mod_name, str(e))), None, sys.exc_info()[2]
>
> This way we will not only see not only exception where was required
> module that was not imported, but also how it was attempted to be
> imported -- often there's cyclic imports, few other imports or
> initializations causing other imports, which are hidden with simple
> raise.
To my mind, this isn't the right approach -- it's papering over the
problem, not fixing it.
In this case, the underlying problem is that ImportError is actually
catching 2 different errors:
1) The view specified doesn't exist
2) The module containing the view contains an error.
The current implementation catches both errors, and turns them into a
ViewDoesNotExist error. You don't fix this problem by providing better
stack trace information on a ViewDoesNotExist error -- you fix the
problem by actually separating the two errors, making (1) return
ViewDoesNotExist, and (2) return ImportError. That way, you'll get a
meaningful error message *and* reliable stack trace data.
Yours,
Russ Magee %-)
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Re: ImportError catching in urlresolvers.py
Hi Russell,
and what do you say about showing call stack properly?
The problem is not ViewDoesNotExist itself, but throwing away useful traceback.
If we do instead:
import sys
try:
self._callback = get_callable(self._callback_str)
except ImportError, e:
mod_name, _ = get_mod_func(self._callback_str)
raise ViewDoesNotExist("Could not import %s. Error was: %s" %
(mod_name, str(e))), None, sys.exc_info()[2]
This way we will not only see not only exception where was required
module that was not imported, but also how it was attempted to be
imported -- often there's cyclic imports, few other imports or
initializations causing other imports, which are hidden with simple
raise.
Few exceptions wrappers patched in my fork:
https://github.com/buriy/django/commit/72b7a7350022a4a0a9cb1f01be62b117dedf89e2.
On Wed, Jun 15, 2011 at 11:46 AM, Russell Keith-Magee
wrote:
> On Wed, Jun 15, 2011 at 11:51 AM, David Cramer wrote:
>> This is currently a problem all over in the Django codebase, and I'd
>> love to see a generic/reusable approach at solving this everywhere.
>
> We already have a generic/reusable approach -- it's just not used
> everywhere that it could be used.
>
> Almost all the places that we are catching ImportError, we're using it
> as part of an implementation of module discovery or dynamic loading.
> In the case of RegexURLPattern, it's the resolution of a string-based
> view specification to a view callable. Catching ImportError is the
> easiest way to establish that a named module doesn't exist, and
> assuming that the code is otherwise sound, it's 100% accurate.
> However, that's a big assumption that doesn't play out in reality, and
> so "couldn't import module due to error in code" errors get confused
> with "module does not exist" errors, leading to the confusion reported
> by the original message.
>
> This is a case where we need to do better at eating our own dogfood.
> Wherever we are using the ImportError trick to catch 'specified module
> does not exist', we should be using a combination of
> django.utils.module_loading.module_has_submodule and
> django.utils.importlib.import_module. Any patch implementing this
> pattern (wherever it occurs) would be looked upon favourably for
> trunk.
>
> Yours,
> Russ Magee %-)
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>
--
Best regards, Yuri V. Baburov, Skype: yuri.baburov, MSN: bu...@live.com
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Re: ImportError catching in urlresolvers.py
On Wed, Jun 15, 2011 at 11:51 AM, David Cramer wrote: > This is currently a problem all over in the Django codebase, and I'd > love to see a generic/reusable approach at solving this everywhere. We already have a generic/reusable approach -- it's just not used everywhere that it could be used. Almost all the places that we are catching ImportError, we're using it as part of an implementation of module discovery or dynamic loading. In the case of RegexURLPattern, it's the resolution of a string-based view specification to a view callable. Catching ImportError is the easiest way to establish that a named module doesn't exist, and assuming that the code is otherwise sound, it's 100% accurate. However, that's a big assumption that doesn't play out in reality, and so "couldn't import module due to error in code" errors get confused with "module does not exist" errors, leading to the confusion reported by the original message. This is a case where we need to do better at eating our own dogfood. Wherever we are using the ImportError trick to catch 'specified module does not exist', we should be using a combination of django.utils.module_loading.module_has_submodule and django.utils.importlib.import_module. Any patch implementing this pattern (wherever it occurs) would be looked upon favourably for trunk. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: ImportError catching in urlresolvers.py
This is currently a problem all over in the Django codebase, and I'd
love to see a generic/reusable approach at solving this everywhere.
On Jun 14, 1:19 pm, Michael Blume wrote:
> In RegexURLPattern._get_callback, we attempt to fetch the callable named by
> the URL pattern, and catch a possible ImportError if this fails. If so, we
> raise ViewDoesNotExist.
>
> try:
> self._callback = get_callable(self._callback_str)
> except ImportError, e:
> mod_name, _ = get_mod_func(self._callback_str)
> raise ViewDoesNotExist("Could not import %s. Error was: %s" %
> (mod_name, str(e)))
>
> The trouble is that the view we're importing may indeed exist, and may
> *itself* make a failed import (say of a model, or some library function), in
> which case ViewDoesNotExist becomes a badly misleading exception name, and
> the resulting traceback is cut unhelpfully short. I've noodled a bit trying
> to come up with a patch, but been stymied by the lack of information bubbled
> up by python's ImportError extension (at least in 2.6).
>
> Personally, I don't think catching ImportError/raising ViewDoesNotExist adds
> much value (we do catch it once in contrib/admindocs/views), so unless we
> can come up with a way to ensure that it only triggers when the top-level
> import fails, I'd recommend eliminating it.
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
ImportError catching in urlresolvers.py
In RegexURLPattern._get_callback, we attempt to fetch the callable named by
the URL pattern, and catch a possible ImportError if this fails. If so, we
raise ViewDoesNotExist.
try:
self._callback = get_callable(self._callback_str)
except ImportError, e:
mod_name, _ = get_mod_func(self._callback_str)
raise ViewDoesNotExist("Could not import %s. Error was: %s" %
(mod_name, str(e)))
The trouble is that the view we're importing may indeed exist, and may
*itself* make a failed import (say of a model, or some library function), in
which case ViewDoesNotExist becomes a badly misleading exception name, and
the resulting traceback is cut unhelpfully short. I've noodled a bit trying
to come up with a patch, but been stymied by the lack of information bubbled
up by python's ImportError extension (at least in 2.6).
Personally, I don't think catching ImportError/raising ViewDoesNotExist adds
much value (we do catch it once in contrib/admindocs/views), so unless we
can come up with a way to ensure that it only triggers when the top-level
import fails, I'd recommend eliminating it.
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
