homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Improve error reporting for packaging.util.resolve_name
Type: behavior Stage: resolved
Components: Distutils2 Versions: Python 3.3, 3rd party
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: Natim, alexis, eric.araujo, eric.snow, erik.bray, tarek
Priority: normal Keywords: easy, patch

Created on 2011年08月06日 10:35 by Natim, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
packaging.util.resolve_name.patch Natim, 2011年08月20日 14:03 Patch the packaging.util.resolve_name to display the right exception. review
test_util.patch Natim, 2011年10月17日 16:09
resolve-name-errors.diff eric.araujo, 2011年10月19日 20:25 review
change-resolve-name.diff eric.araujo, 2011年11月20日 15:19 review
Repositories containing patches
https://bitbucket.org/natim/cpython
https://bitbucket.org/natim/cpython
Messages (18)
msg141715 - (view) Author: Rémy HUBSCHER (Natim) * Date: 2011年08月06日 10:35
I patched it for redbarrel but there is the same bug on distutils2.
This patch allow to see the error raised when loading the module instead of saying that the module does not exists.
https://bitbucket.org/tarek/redbarrel/changeset/2cf6d7e32081#chg-redbarrel/util.py 
msg141717 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年08月06日 14:41
Thanks for the report. Would you have time to make a code+test patch for the CPython repo? distutils2 has moved there under the name packaging.
msg142533 - (view) Author: Rémy HUBSCHER (Natim) * Date: 2011年08月20日 14:03
Hello, I did the patch, but I have no idea of how to make a test for it.
More over, I have seen a similar problem each time there is this code in the Python code (here in distutils.util.Distribution.get_command_class) :
 try:
 __import__ (module_name)
 module = sys.modules[module_name]
 except ImportError:
 continue
 try:
 klass = getattr(module, klass_name)
 except AttributeError:
 raise DistutilsModuleError(
 "invalid command '%s' (no class '%s' in module '%s')"
 % (command, klass_name, module_name))
Maybe it could be better to have a function in cpython to do that ?
msg142534 - (view) Author: Rémy HUBSCHER (Natim) * Date: 2011年08月20日 14:23
Actually it is not the same problem for `distutils.util.Distribution.get_command_class` my mistake.
msg142535 - (view) Author: Alexis Metaireau (alexis) * (Python triager) Date: 2011年08月20日 15:03
Thanks Rémy, 
About testing, I would go for modules with errors in it and check that when imported trough this function it does what it is supposed to do.
IOW:
1. Create a test python module with errors in their definition (Throw an exception would do the trick)
2a. Try to load this python module, check that the exception is the one thrown in the module
2.b Check that loading a non existing modules still tell us that the module doesn't exist
2.c Check that importing something existing works, trough this function. 
Hope I'm clear enough, thanks again,
Alexis
msg142601 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年08月21日 09:24
I’m confused about what the proposed change would actually do. Could you explain what behavior you want to change and how?
msg142602 - (view) Author: Rémy HUBSCHER (Natim) * Date: 2011年08月21日 09:28
It is exactly what explained Alexis.
The __import__ can failed in two case :
 - The module doesn't exist
 - There is a error in the module
With the previous behaviour, even if the module exist, the message was that it doesn't exists. And it was then not fast forward to guess where was the error.
With this new behaviour, if there is an __import__ error and the module file actually exists, then we raise the real exception problem from the module that we try to import.
msg145681 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年10月17日 12:35
I would like to commit this. Tests are needed. Does someone want to write them? Please ask any question you might have, we’re here to help :)
msg145696 - (view) Author: Rémy HUBSCHER (Natim) * Date: 2011年10月17日 13:54
Ok, I am working on it.
msg145722 - (view) Author: Rémy HUBSCHER (Natim) * Date: 2011年10月17日 16:09
Here is the patch for the test case.
msg145829 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年10月18日 16:09
Thanks. I’ll add more tests and commit. (I also prefer to create modules in test methods instead of using another file, so that I can see right here what the module contains.)
msg145955 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年10月19日 20:25
I have added tests and fixed one or two bugs in 1405df4a1535. I have another patch that checks that error messages are useful, with one exception: if you try to import a.b and b raises an ImportError, then the calling code will see an Attribute error. Right now I don’t see how we can avoid that; checking the existence of files like your patch proposes is not possible, as Python modules do not come from files.
msg147996 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年11月20日 15:19
I’ve found a way to make sure error messages always bubble up *and* clean up the ugly code in resolve_name, but it’s a rather drastic one. The idea is to restrict the function to work only with names defined at the module level, not arbitrarily nested names. That way, the code is much easier to write: split on '.', pop the last element and keep it for later, __import__ the rest of the name and look it up in sys.modules. One consequence is that you can’t use package.module.SomeClass.NestedClass for a command, nor module.SomeClass.staticmethod as setup hook; to be pragmatic, I’m not sure that was really useful, and in any case you just have to do a module-level alias ("x = SomeClass.staticmethod") to make it work.
To reflect the fact that the function has restrictions, I renamed it from the generic "resolve_name" to the vague "find_object"; vague is better because it makes less promises and should cause developers using to look at the docs or docstring.
In short, it’s a clear improvement code-wise that should not impact most of the users, and I like it a lot.
msg152750 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012年02月06日 17:18
In the absence of feedback, I’m going to apply my find_object idea as described in my previous message.
msg152865 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012年02月08日 15:02
FYI, here is code that can handle arbitrary dotted names: <http://svn.eby-sarna.com/Importing/peak/util/imports.py?view=markup>. I haven’t checked if its error reporting has the problem we’re discussing in this report.
An alternative would be to use colon notation, e.g. package.submodule:Thing.Nested.attribute.
My preference is still for find_object, using dots and with the nesting prohibition.
msg164566 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012年07月03日 04:36
http://bugs.python.org/file25773/resolve_name.patch is a patch that cleans up the function; I’ll see if it can be used to solve our problems without having to follow my drastic find_object idea.
msg164567 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012年07月03日 04:37
BTW modules in the standard library all use the dotted notation AFAIK, not the colon notation, so I would strongly prefer avoiding the colon notation.
msg213233 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014年03月12日 09:48
This is now obsolete. For a discussion about moving resolve_name to another part of the stdlib, see #12915.
History
Date User Action Args
2022年04月11日 14:57:20adminsetgithub: 56912
2014年03月12日 09:48:28eric.araujosetstatus: open -> closed
resolution: out of date
messages: + msg213233

stage: patch review -> resolved
2012年11月13日 04:31:32eric.snowsetnosy: + eric.snow
2012年07月03日 04:37:39eric.araujosetmessages: + msg164567
2012年07月03日 04:36:29eric.araujosetmessages: + msg164566
2012年05月31日 12:03:42tareklinkissue14967 superseder
2012年02月08日 15:02:42eric.araujosetnosy: + erik.bray
messages: + msg152865
2012年02月06日 17:18:22eric.araujosetmessages: + msg152750
2011年11月20日 15:19:34eric.araujosetfiles: + change-resolve-name.diff

messages: + msg147996
stage: test needed -> patch review
2011年10月19日 20:25:09eric.araujosetfiles: + resolve-name-errors.diff

messages: + msg145955
2011年10月18日 16:44:24eric.araujolinkissue13172 dependencies
2011年10月18日 16:09:24eric.araujosetmessages: + msg145829
2011年10月17日 16:09:31Natimsetfiles: + test_util.patch
hgrepos: + hgrepo84
messages: + msg145722
2011年10月17日 13:54:42Natimsetmessages: + msg145696
2011年10月17日 12:35:22eric.araujosetstage: needs patch -> test needed
messages: + msg145681
versions: + 3rd party
2011年08月21日 09:28:58Natimsetmessages: + msg142602
2011年08月21日 09:24:07eric.araujosetmessages: + msg142601
2011年08月20日 15:03:12alexissetmessages: + msg142535
2011年08月20日 14:23:51Natimsetmessages: + msg142534
2011年08月20日 14:03:39Natimsetfiles: + packaging.util.resolve_name.patch
hgrepos: + hgrepo59
messages: + msg142533

keywords: + patch
2011年08月19日 12:45:55eric.araujosetkeywords: + easy
2011年08月06日 14:41:05eric.araujosetversions: + Python 3.3
title: Loading a module -> Improve error reporting for packaging.util.resolve_name
messages: + msg141717

assignee: tarek -> eric.araujo
type: behavior
stage: needs patch
2011年08月06日 10:35:35Natimcreate

AltStyle によって変換されたページ (->オリジナル) /