Launchpad itself

Merge lp:~maxb/launchpad/py2.6-importfascist-again into lp:launchpad

  1. py2.6-importfascist-again
  2. Merge into devel
Proposed by Max Bowsher
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~maxb/launchpad/py2.6-importfascist-again
Merge into: lp:launchpad
Diff against target: 116 lines (+24/-21)
1 file modified
lib/lp/scripts/utilities/importfascist.py (+24/-21)
To merge this branch: bzr merge lp:~maxb/launchpad/py2.6-importfascist-again
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+19927@code.launchpad.net

Commit message

Fix the argument names of importfascist to match those of the default __import__, as those are actually part of the interface when called by keyword arguments.

To post a comment you must log in.
Revision history for this message
Max Bowsher (maxb) wrote :

Back in devel r10090, jml renamed some of the parameters of importfascist to "nicer" names. However, these names are actually part of the interface of __import__(...) when invoked using keyword arguments, and the encodings module in the Python 2.6 standard library actually does so. So, this branch puts the names back to the standard ones as defined by the built-it __import__(...).

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Sigh. Looks ok though.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/scripts/utilities/importfascist.py'
2--- lib/lp/scripts/utilities/importfascist.py 2010-02-04 03:07:25 +0000
3+++ lib/lp/scripts/utilities/importfascist.py 2010-02-23 01:07:17 +0000
4@@ -173,12 +173,15 @@
5 % self.import_into)
6
7
8+# The names of the arguments form part of the interface of __import__(...), and
9+# must not be changed, as code may choose to invoke __import__ using keyword
10+# arguments - e.g. the encodings module in Python 2.6.
11 # pylint: disable-msg=W0102,W0602
12-def import_fascist(module_name, globals={}, locals={}, from_list=[], level=-1):
13+def import_fascist(name, globals={}, locals={}, fromlist=[], level=-1):
14 global naughty_imports
15
16 try:
17- module = original_import(module_name, globals, locals, from_list, level)
18+ module = original_import(name, globals, locals, fromlist, level)
19 except ImportError:
20 # XXX sinzui 2008-04-17 bug=277274:
21 # import_fascist screws zope configuration module which introspects
22@@ -188,18 +191,18 @@
23 # time doesn't exist and dies a horrible death because of the import
24 # fascist. That's the long explanation for why we special case this
25 # module.
26- if module_name.startswith('zope.app.layers.'):
27- module_name = module_name[16:]
28- module = original_import(module_name, globals, locals, from_list, level)
29+ if name.startswith('zope.app.layers.'):
30+ name = name[16:]
31+ module = original_import(name, globals, locals, fromlist, level)
32 else:
33 raise
34 # Python's re module imports some odd stuff every time certain regexes
35 # are used. Let's optimize this.
36- if module_name == 'sre':
37+ if name == 'sre':
38 return module
39
40 # Mailman 2.1 code base is originally circa 1998, so yeah, no __all__'s.
41- if module_name.startswith('Mailman'):
42+ if name.startswith('Mailman'):
43 return module
44
45 # Some uses of __import__ pass None for globals, so handle that.
46@@ -215,14 +218,14 @@
47
48 # Check the "NotFoundError" policy.
49 if (import_into.startswith('canonical.launchpad.database') and
50- module_name == 'zope.exceptions'):
51- if from_list and 'NotFoundError' in from_list:
52+ name == 'zope.exceptions'):
53+ if fromlist and 'NotFoundError' in fromlist:
54 raise NotFoundPolicyViolation(import_into)
55
56 # Check the database import policy.
57- if (module_name.startswith(database_root) and
58+ if (name.startswith(database_root) and
59 not database_import_allowed_into(import_into)):
60- error = DatabaseImportPolicyViolation(import_into, module_name)
61+ error = DatabaseImportPolicyViolation(import_into, name)
62 naughty_imports.add(error)
63 # Raise an error except in the case of browser.traversers.
64 # This exception to raising an error is only temporary, until
65@@ -231,28 +234,28 @@
66 raise error
67
68 # Check the import from __all__ policy.
69- if from_list is not None and (
70+ if fromlist is not None and (
71 import_into.startswith('canonical') or import_into.startswith('lp')):
72 # We only want to warn about "from foo import bar" violations in our
73 # own code.
74- from_list = list(from_list)
75+ fromlist = list(fromlist)
76 module_all = getattr(module, '__all__', None)
77 if module_all is None:
78- if from_list == ['*']:
79+ if fromlist == ['*']:
80 # "from foo import *" is naughty if foo has no __all__
81- error = FromStarPolicyViolation(import_into, module_name)
82+ error = FromStarPolicyViolation(import_into, name)
83 naughty_imports.add(error)
84 raise error
85 else:
86- if from_list == ['*']:
87+ if fromlist == ['*']:
88 # "from foo import *" is allowed if foo has an __all__
89 return module
90 if is_test_module(import_into):
91 # We don't bother checking imports into test modules.
92 return module
93- allowed_from_list = valid_imports_not_in_all.get(
94- module_name, set())
95- for attrname in from_list:
96+ allowed_fromlist = valid_imports_not_in_all.get(
97+ name, set())
98+ for attrname in fromlist:
99 # Check that each thing we are importing into the module is
100 # either in __all__, is a module itself, or is a specific
101 # exception.
102@@ -264,13 +267,13 @@
103 # You can import modules even when they aren't declared in
104 # __all__.
105 continue
106- if attrname in allowed_from_list:
107+ if attrname in allowed_fromlist:
108 # Some things can be imported even if they aren't in
109 # __all__.
110 continue
111 if attrname not in module_all:
112 error = NotInModuleAllPolicyViolation(
113- import_into, module_name, attrname)
114+ import_into, name, attrname)
115 naughty_imports.add(error)
116 return module
117