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
=== modified file 'lib/lp/scripts/utilities/importfascist.py'
--- lib/lp/scripts/utilities/importfascist.py 2010-02-04 03:07:25 +0000
+++ lib/lp/scripts/utilities/importfascist.py 2010-02-23 01:07:17 +0000
@@ -173,12 +173,15 @@
173 % self.import_into)173 % self.import_into)
174174
175175
176# The names of the arguments form part of the interface of __import__(...), and
177# must not be changed, as code may choose to invoke __import__ using keyword
178# arguments - e.g. the encodings module in Python 2.6.
176# pylint: disable-msg=W0102,W0602179# pylint: disable-msg=W0102,W0602
177def import_fascist(module_name, globals={}, locals={}, from_list=[], level=-1):180def import_fascist(name, globals={}, locals={}, fromlist=[], level=-1):
178 global naughty_imports181 global naughty_imports
179182
180 try:183 try:
181 module = original_import(module_name, globals, locals, from_list, level)184 module = original_import(name, globals, locals, fromlist, level)
182 except ImportError:185 except ImportError:
183 # XXX sinzui 2008-04-17 bug=277274:186 # XXX sinzui 2008-04-17 bug=277274:
184 # import_fascist screws zope configuration module which introspects187 # import_fascist screws zope configuration module which introspects
@@ -188,18 +191,18 @@
188 # time doesn't exist and dies a horrible death because of the import191 # time doesn't exist and dies a horrible death because of the import
189 # fascist. That's the long explanation for why we special case this192 # fascist. That's the long explanation for why we special case this
190 # module.193 # module.
191 if module_name.startswith('zope.app.layers.'):194 if name.startswith('zope.app.layers.'):
192 module_name = module_name[16:]195 name = name[16:]
193 module = original_import(module_name, globals, locals, from_list, level)196 module = original_import(name, globals, locals, fromlist, level)
194 else:197 else:
195 raise198 raise
196 # Python's re module imports some odd stuff every time certain regexes199 # Python's re module imports some odd stuff every time certain regexes
197 # are used. Let's optimize this.200 # are used. Let's optimize this.
198 if module_name == 'sre':201 if name == 'sre':
199 return module202 return module
200203
201 # Mailman 2.1 code base is originally circa 1998, so yeah, no __all__'s.204 # Mailman 2.1 code base is originally circa 1998, so yeah, no __all__'s.
202 if module_name.startswith('Mailman'):205 if name.startswith('Mailman'):
203 return module206 return module
204207
205 # Some uses of __import__ pass None for globals, so handle that.208 # Some uses of __import__ pass None for globals, so handle that.
@@ -215,14 +218,14 @@
215218
216 # Check the "NotFoundError" policy.219 # Check the "NotFoundError" policy.
217 if (import_into.startswith('canonical.launchpad.database') and220 if (import_into.startswith('canonical.launchpad.database') and
218 module_name == 'zope.exceptions'):221 name == 'zope.exceptions'):
219 if from_list and 'NotFoundError' in from_list:222 if fromlist and 'NotFoundError' in fromlist:
220 raise NotFoundPolicyViolation(import_into)223 raise NotFoundPolicyViolation(import_into)
221224
222 # Check the database import policy.225 # Check the database import policy.
223 if (module_name.startswith(database_root) and226 if (name.startswith(database_root) and
224 not database_import_allowed_into(import_into)):227 not database_import_allowed_into(import_into)):
225 error = DatabaseImportPolicyViolation(import_into, module_name)228 error = DatabaseImportPolicyViolation(import_into, name)
226 naughty_imports.add(error)229 naughty_imports.add(error)
227 # Raise an error except in the case of browser.traversers.230 # Raise an error except in the case of browser.traversers.
228 # This exception to raising an error is only temporary, until231 # This exception to raising an error is only temporary, until
@@ -231,28 +234,28 @@
231 raise error234 raise error
232235
233 # Check the import from __all__ policy.236 # Check the import from __all__ policy.
234 if from_list is not None and (237 if fromlist is not None and (
235 import_into.startswith('canonical') or import_into.startswith('lp')):238 import_into.startswith('canonical') or import_into.startswith('lp')):
236 # We only want to warn about "from foo import bar" violations in our239 # We only want to warn about "from foo import bar" violations in our
237 # own code.240 # own code.
238 from_list = list(from_list)241 fromlist = list(fromlist)
239 module_all = getattr(module, '__all__', None)242 module_all = getattr(module, '__all__', None)
240 if module_all is None:243 if module_all is None:
241 if from_list == ['*']:244 if fromlist == ['*']:
242 # "from foo import *" is naughty if foo has no __all__245 # "from foo import *" is naughty if foo has no __all__
243 error = FromStarPolicyViolation(import_into, module_name)246 error = FromStarPolicyViolation(import_into, name)
244 naughty_imports.add(error)247 naughty_imports.add(error)
245 raise error248 raise error
246 else:249 else:
247 if from_list == ['*']:250 if fromlist == ['*']:
248 # "from foo import *" is allowed if foo has an __all__251 # "from foo import *" is allowed if foo has an __all__
249 return module252 return module
250 if is_test_module(import_into):253 if is_test_module(import_into):
251 # We don't bother checking imports into test modules.254 # We don't bother checking imports into test modules.
252 return module255 return module
253 allowed_from_list = valid_imports_not_in_all.get(256 allowed_fromlist = valid_imports_not_in_all.get(
254 module_name, set())257 name, set())
255 for attrname in from_list:258 for attrname in fromlist:
256 # Check that each thing we are importing into the module is259 # Check that each thing we are importing into the module is
257 # either in __all__, is a module itself, or is a specific260 # either in __all__, is a module itself, or is a specific
258 # exception.261 # exception.
@@ -264,13 +267,13 @@
264 # You can import modules even when they aren't declared in267 # You can import modules even when they aren't declared in
265 # __all__.268 # __all__.
266 continue269 continue
267 if attrname in allowed_from_list:270 if attrname in allowed_fromlist:
268 # Some things can be imported even if they aren't in271 # Some things can be imported even if they aren't in
269 # __all__.272 # __all__.
270 continue273 continue
271 if attrname not in module_all:274 if attrname not in module_all:
272 error = NotInModuleAllPolicyViolation(275 error = NotInModuleAllPolicyViolation(
273 import_into, module_name, attrname)276 import_into, name, attrname)
274 naughty_imports.add(error)277 naughty_imports.add(error)
275 return module278 return module
276279