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 %-)
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.
> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@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
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 %-)
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
Looks good to me! I've just marked to RFC. Thanks for the patch Bas!
Yours,
Russ Magee %-)