Launchpad itself

Merge lp:~jml/launchpad/import-fascist-tweaks into lp:launchpad

  1. import-fascist-tweaks
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Muharem Hrnjadovic
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~jml/launchpad/import-fascist-tweaks
Merge into: lp:launchpad
Diff against target: 375 lines (+90/-54)
13 files modified
lib/canonical/launchpad/fields/__init__.py (+2/-0)
lib/lp/bugs/model/bugwatch.py (+1/-2)
lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py (+1/-0)
lib/lp/registry/interfaces/mailinglist.py (+1/-1)
lib/lp/registry/interfaces/teammembership.py (+1/-0)
lib/lp/registry/model/structuralsubscription.py (+3/-1)
lib/lp/scripts/utilities/importfascist.py (+66/-39)
lib/lp/services/job/interfaces/job.py (+1/-0)
lib/lp/soyuz/model/archive.py (+8/-4)
lib/lp/soyuz/model/archivesubscriber.py (+2/-2)
lib/lp/soyuz/model/builder.py (+2/-3)
lib/lp/soyuz/model/distroarchseries.py (+1/-2)
lib/lp/soyuz/model/files.py (+1/-0)
To merge this branch: bzr merge lp:~jml/launchpad/import-fascist-tweaks
Reviewer Review Type Date Requested Status
Muharem Hrnjadovic (community) Approve
Review via email: mp+16560@code.launchpad.net

Commit message

importfascist now checks 'lp' package and is now more data-driven. Many dodgy imports fixed.

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

This branch does a few different things, all to do with the import fascist. The patch:
  * restructures the logic to be a bit clearer.
  * provides a data-driven approach for allowing imports that aren't in __all__
  * makes importfascist check packages in 'lp' and not just 'canonical'
  * stops the warnings that we were all getting when running the tests
  * fixes a whole bunch of warnings that we weren't getting because of 'lp' being ignored
  * cleans up random pyflakes

Merry Christmas

Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Nice branch, approved with the changes in http://pastebin.ubuntu.com/345781/

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/fields/__init__.py'
2--- lib/canonical/launchpad/fields/__init__.py 2009-12-09 23:11:31 +0000
3+++ lib/canonical/launchpad/fields/__init__.py 2009-12-24 08:18:19 +0000
4@@ -32,9 +32,11 @@
5 'LocationField',
6 'LogoImageUpload',
7 'MugshotImageUpload',
8+ 'NoneableDescription',
9 'NoneableTextLine',
10 'ParticipatingPersonChoice',
11 'PasswordField',
12+ 'PersonChoice',
13 'PillarAliases',
14 'PillarNameField',
15 'PrivateMembershipTeamNotAllowed',
16
17=== modified file 'lib/lp/bugs/model/bugwatch.py'
18--- lib/lp/bugs/model/bugwatch.py 2009-12-15 17:28:51 +0000
19+++ lib/lp/bugs/model/bugwatch.py 2009-12-24 08:18:19 +0000
20@@ -35,10 +35,9 @@
21 from canonical.launchpad.webapp import urlappend, urlsplit
22 from canonical.launchpad.webapp.interfaces import NotFoundError
23
24-from lp.bugs.interfaces.bug import IBugWatch
25 from lp.bugs.interfaces.bugtracker import BugTrackerType, IBugTrackerSet
26 from lp.bugs.interfaces.bugwatch import (
27- BugWatchErrorType, IBugWatchSet, NoBugTrackerFound,
28+ BugWatchErrorType, IBugWatch, IBugWatchSet, NoBugTrackerFound,
29 UnrecognizedBugTrackerURL)
30 from lp.bugs.model.bugmessage import BugMessage
31 from lp.bugs.model.bugset import BugSetBase
32
33=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py'
34--- lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2009-12-03 14:38:48 +0000
35+++ lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2009-12-24 08:18:19 +0000
36@@ -8,6 +8,7 @@
37 __metaclass__ = type
38
39 __all__ = [
40+ 'BuildBehaviorMismatch',
41 'IBuildFarmJobBehavior',
42 ]
43
44
45=== modified file 'lib/lp/registry/interfaces/mailinglist.py'
46--- lib/lp/registry/interfaces/mailinglist.py 2009-12-03 19:12:02 +0000
47+++ lib/lp/registry/interfaces/mailinglist.py 2009-12-24 08:18:19 +0000
48@@ -31,7 +31,7 @@
49
50 from canonical.launchpad import _
51 from canonical.launchpad.fields import PublicPersonChoice
52-from lp.registry.interfaces.person import IEmailAddress
53+from canonical.launchpad.interfaces.emailaddress import IEmailAddress
54 from canonical.launchpad.interfaces.librarian import ILibraryFileAlias
55 from canonical.launchpad.interfaces.message import IMessage
56 from lp.registry.interfaces.person import IPerson
57
58=== modified file 'lib/lp/registry/interfaces/teammembership.py'
59--- lib/lp/registry/interfaces/teammembership.py 2009-12-19 22:19:12 +0000
60+++ lib/lp/registry/interfaces/teammembership.py 2009-12-24 08:18:19 +0000
61@@ -14,6 +14,7 @@
62 'ITeamMembershipSet',
63 'ITeamParticipation',
64 'TeamMembershipStatus',
65+ 'UserCannotChangeMembershipSilently',
66 ]
67
68 from zope.schema import Bool, Choice, Datetime, Int, Text
69
70=== modified file 'lib/lp/registry/model/structuralsubscription.py'
71--- lib/lp/registry/model/structuralsubscription.py 2009-12-05 18:37:28 +0000
72+++ lib/lp/registry/model/structuralsubscription.py 2009-12-24 08:18:19 +0000
73@@ -17,9 +17,11 @@
74
75 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
76 from lp.registry.interfaces.distribution import IDistribution
77+from lp.registry.interfaces.distributionsourcepackage import (
78+ IDistributionSourcePackage)
79 from lp.registry.interfaces.distroseries import IDistroSeries
80 from lp.registry.interfaces.milestone import IMilestone
81-from lp.registry.interfaces.product import IDistributionSourcePackage, IProduct
82+from lp.registry.interfaces.product import IProduct
83 from lp.registry.interfaces.productseries import IProductSeries
84 from lp.registry.interfaces.project import IProject
85 from lp.registry.interfaces.structuralsubscription import (
86
87=== modified file 'lib/lp/scripts/utilities/importfascist.py'
88--- lib/lp/scripts/utilities/importfascist.py 2009-08-27 01:32:01 +0000
89+++ lib/lp/scripts/utilities/importfascist.py 2009-12-24 08:18:19 +0000
90@@ -52,6 +52,17 @@
91 """)
92
93
94+# Sometimes, third-party modules don't export all of their public APIs through
95+# __all__. The following dict maps from such modules to a list of attributes
96+# that are allowed to be imported, whether or not they are in __all__.
97+valid_imports_not_in_all = {
98+ 'cookielib': set(['domain_match']),
99+ 'email.Utils': set(['mktime_tz']),
100+ 'textwrap': set(['dedent']),
101+ 'zope.component': set(['adapter', 'provideHandler']),
102+ }
103+
104+
105 def database_import_allowed_into(module_path):
106 """Return True if database code is allowed to be imported into the given
107 module path. Otherwise, returns False.
108@@ -156,11 +167,11 @@
109
110
111 # pylint: disable-msg=W0102,W0602
112-def import_fascist(name, globals={}, locals={}, fromlist=[]):
113+def import_fascist(module_name, globals={}, locals={}, from_list=[]):
114 global naughty_imports
115
116 try:
117- module = original_import(name, globals, locals, fromlist)
118+ module = original_import(module_name, globals, locals, from_list)
119 except ImportError:
120 # XXX sinzui 2008-04-17 bug=277274:
121 # import_fascist screws zope configuration module which introspects
122@@ -170,19 +181,18 @@
123 # time doesn't exist and dies a horrible death because of the import
124 # fascist. That's the long explanation for why we special case this
125 # module.
126- if name.startswith('zope.app.layers.'):
127- name = name[16:]
128- module = original_import(name, globals, locals, fromlist)
129+ if module_name.startswith('zope.app.layers.'):
130+ module_name = module_name[16:]
131+ module = original_import(module_name, globals, locals, from_list)
132 else:
133 raise
134 # Python's re module imports some odd stuff every time certain regexes
135 # are used. Let's optimize this.
136- # Also, 'dedent' is not in textwrap.__all__.
137- if name == 'sre' or name == 'textwrap':
138+ if module_name == 'sre':
139 return module
140
141 # Mailman 2.1 code base is originally circa 1998, so yeah, no __all__'s.
142- if name.startswith('Mailman'):
143+ if module_name.startswith('Mailman'):
144 return module
145
146 # Some uses of __import__ pass None for globals, so handle that.
147@@ -195,13 +205,17 @@
148 # We could find out by jumping up the stack a frame.
149 # Let's not for now.
150 import_into = '__import__ hook'
151+
152+ # Check the "NotFoundError" policy.
153 if (import_into.startswith('canonical.launchpad.database') and
154- name == 'zope.exceptions'):
155- if fromlist and 'NotFoundError' in fromlist:
156+ module_name == 'zope.exceptions'):
157+ if from_list and 'NotFoundError' in from_list:
158 raise NotFoundPolicyViolation(import_into)
159- if (name.startswith(database_root) and
160+
161+ # Check the database import policy.
162+ if (module_name.startswith(database_root) and
163 not database_import_allowed_into(import_into)):
164- error = DatabaseImportPolicyViolation(import_into, name)
165+ error = DatabaseImportPolicyViolation(import_into, module_name)
166 naughty_imports.add(error)
167 # Raise an error except in the case of browser.traversers.
168 # This exception to raising an error is only temporary, until
169@@ -209,34 +223,48 @@
170 if import_into not in warned_database_imports:
171 raise error
172
173- if fromlist is not None and import_into.startswith('canonical'):
174+ # Check the import from __all__ policy.
175+ if from_list is not None and (
176+ import_into.startswith('canonical') or import_into.startswith('lp')):
177 # We only want to warn about "from foo import bar" violations in our
178 # own code.
179- if list(fromlist) == ['*'] and not hasattr(module, '__all__'):
180- # "from foo import *" is naughty if foo has no __all__
181- error = FromStarPolicyViolation(import_into, name)
182- naughty_imports.add(error)
183- raise error
184- elif (list(fromlist) != ['*'] and hasattr(module, '__all__') and
185- not is_test_module(import_into)):
186- # "from foo import bar" is naughty if bar isn't in foo.__all__
187- # (and foo actually has an __all__). Unless foo is within a tests
188- # or ftests module or bar is itself a module.
189- for attrname in fromlist:
190- if (attrname in ('adapter', 'provideHandler')
191- and module.__name__ == 'zope.component'):
192- # 'adapter' and 'provideHandler' are not in
193- # zope.component.__all__, but that's where they should be
194- # imported from.
195- continue
196- if attrname != '__doc__' and attrname not in module.__all__:
197- if not isinstance(
198- getattr(module, attrname, None), types.ModuleType):
199- error = NotInModuleAllPolicyViolation(
200- import_into, name, attrname)
201- naughty_imports.add(error)
202- # Not raising on NotInModuleAllPolicyViolation yet.
203- #raise error
204+ from_list = list(from_list)
205+ module_all = getattr(module, '__all__', None)
206+ if module_all is None:
207+ if from_list == ['*']:
208+ # "from foo import *" is naughty if foo has no __all__
209+ error = FromStarPolicyViolation(import_into, module_name)
210+ naughty_imports.add(error)
211+ raise error
212+ else:
213+ if from_list == ['*']:
214+ # "from foo import *" is allowed if foo has an __all__
215+ return module
216+ if is_test_module(import_into):
217+ # We don't bother checking imports into test modules.
218+ return module
219+ allowed_from_list = valid_imports_not_in_all.get(
220+ module_name, set())
221+ for attrname in from_list:
222+ # Check that each thing we are importing into the module is
223+ # either in __all__, is a module itself, or is a specific
224+ # exception.
225+ if attrname == '__doc__':
226+ # You can always import __doc__.
227+ continue
228+ if isinstance(
229+ getattr(module, attrname, None), types.ModuleType):
230+ # You can import modules even when they aren't declared in
231+ # __all__.
232+ continue
233+ if attrname in allowed_from_list:
234+ # Some things can be imported even if they aren't in
235+ # __all__.
236+ continue
237+ if attrname not in module_all:
238+ error = NotInModuleAllPolicyViolation(
239+ import_into, module_name, attrname)
240+ naughty_imports.add(error)
241 return module
242
243
244@@ -244,7 +272,6 @@
245 if naughty_imports:
246 print
247 print '** %d import policy violations **' % len(naughty_imports)
248- current_type = None
249
250 database_violations = []
251 fromstar_violations = []
252
253=== modified file 'lib/lp/services/job/interfaces/job.py'
254--- lib/lp/services/job/interfaces/job.py 2009-09-02 21:09:48 +0000
255+++ lib/lp/services/job/interfaces/job.py 2009-12-24 08:18:19 +0000
256@@ -9,6 +9,7 @@
257
258 __all__ = [
259 'IJob',
260+ 'IRunnableJob',
261 'JobStatus',
262 'LeaseHeld',
263 ]
264
265=== modified file 'lib/lp/soyuz/model/archive.py'
266--- lib/lp/soyuz/model/archive.py 2009-12-08 15:40:23 +0000
267+++ lib/lp/soyuz/model/archive.py 2009-12-24 08:18:19 +0000
268@@ -54,7 +54,7 @@
269 from lp.registry.model.teammembership import TeamParticipation
270 from lp.soyuz.interfaces.archive import (
271 AlreadySubscribed, ArchiveDependencyError, ArchiveNotPrivate,
272- ArchivePurpose, DistroSeriesNotFound, IArchive, IArchiveSet,
273+ ArchivePurpose, CannotCopy, DistroSeriesNotFound, IArchive, IArchiveSet,
274 IDistributionArchive, InvalidComponent, IPPA, MAIN_ARCHIVE_PURPOSES,
275 NoSuchPPA, PocketNotFound, VersionRequiresName, default_name_by_purpose)
276 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
277@@ -77,7 +77,7 @@
278 from lp.soyuz.interfaces.publishing import (
279 active_publishing_status, PackagePublishingStatus, IPublishingSet)
280 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
281-from lp.soyuz.scripts.packagecopier import CannotCopy, do_copy
282+from lp.soyuz.scripts.packagecopier import do_copy
283 from canonical.launchpad.webapp.interfaces import (
284 IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
285 from canonical.launchpad.webapp.url import urlappend
286@@ -1096,7 +1096,9 @@
287 # Find and validate the source package names in source_names.
288 sources = []
289 for name in source_names:
290- source_package_name = getUtility(ISourcePackageNameSet)[name]
291+ # Check to see if the source package exists, and raise a useful
292+ # error if it doesn't.
293+ getUtility(ISourcePackageNameSet)[name]
294 # Grabbing the item at index 0 ensures it's the most recent
295 # publication.
296 sources.append(
297@@ -1108,8 +1110,10 @@
298 def syncSource(self, source_name, version, from_archive, to_pocket,
299 to_series=None, include_binaries=False):
300 """See `IArchive`."""
301+ # Check to see if the source package exists, and raise a useful error
302+ # if it doesn't.
303+ getUtility(ISourcePackageNameSet)[source_name]
304 # Find and validate the source package version required.
305- source_package_name = getUtility(ISourcePackageNameSet)[source_name]
306 source = from_archive.getPublishedSources(
307 name=source_name, version=version, exact_match=True)[0]
308
309
310=== modified file 'lib/lp/soyuz/model/archivesubscriber.py'
311--- lib/lp/soyuz/model/archivesubscriber.py 2009-07-17 00:26:05 +0000
312+++ lib/lp/soyuz/model/archivesubscriber.py 2009-12-24 08:18:19 +0000
313@@ -20,8 +20,8 @@
314
315 from canonical.database.constants import UTC_NOW
316 from canonical.database.enumcol import DBEnum
317-from lp.soyuz.model.archiveauthtoken import (
318- ArchiveAuthToken, IArchiveAuthTokenSet)
319+from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
320+from lp.soyuz.model.archiveauthtoken import ArchiveAuthToken
321 from lp.registry.interfaces.person import (
322 validate_person_not_private_membership)
323 from lp.registry.model.teammembership import TeamParticipation
324
325=== modified file 'lib/lp/soyuz/model/builder.py'
326--- lib/lp/soyuz/model/builder.py 2009-12-04 12:49:25 +0000
327+++ lib/lp/soyuz/model/builder.py 2009-12-24 08:18:19 +0000
328@@ -42,8 +42,7 @@
329 from lp.registry.interfaces.person import validate_public_person
330 from lp.registry.interfaces.pocket import PackagePublishingPocket
331 from canonical.launchpad.helpers import filenameToContentType
332-from canonical.launchpad.interfaces._schema_circular_imports import (
333- IHasBuildRecords)
334+from lp.soyuz.interfaces.buildrecords import IHasBuildRecords
335 from lp.soyuz.interfaces.distroarchseries import IDistroArchSeriesSet
336 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
337 from canonical.launchpad.webapp.interfaces import NotFoundError
338@@ -380,7 +379,7 @@
339 return False
340 try:
341 slavestatus = self.slaveStatusSentence()
342- except (xmlrpclib.Fault, socket.error), info:
343+ except (xmlrpclib.Fault, socket.error):
344 return False
345 if slavestatus[0] != BuilderStatus.IDLE:
346 return False
347
348=== modified file 'lib/lp/soyuz/model/distroarchseries.py'
349--- lib/lp/soyuz/model/distroarchseries.py 2009-08-28 07:34:44 +0000
350+++ lib/lp/soyuz/model/distroarchseries.py 2009-12-24 08:18:19 +0000
351@@ -23,12 +23,11 @@
352
353 from canonical.launchpad.components.decoratedresultset import (
354 DecoratedResultSet)
355-from canonical.launchpad.interfaces._schema_circular_imports import (
356- IHasBuildRecords)
357 from lp.registry.interfaces.pocket import PackagePublishingPocket
358 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageName
359 from lp.soyuz.interfaces.binarypackagerelease import IBinaryPackageReleaseSet
360 from lp.soyuz.interfaces.build import IBuildSet
361+from lp.soyuz.interfaces.buildrecords import IHasBuildRecords
362 from lp.soyuz.interfaces.distroarchseries import (
363 IDistroArchSeries, IDistroArchSeriesSet, IPocketChroot)
364 from lp.soyuz.interfaces.publishing import (
365
366=== modified file 'lib/lp/soyuz/model/files.py'
367--- lib/lp/soyuz/model/files.py 2009-11-25 23:41:34 +0000
368+++ lib/lp/soyuz/model/files.py 2009-12-24 08:18:19 +0000
369@@ -7,6 +7,7 @@
370 __all__ = [
371 'BinaryPackageFile',
372 'BinaryPackageFileSet',
373+ 'SourceFileMixin',
374 'SourcePackageReleaseFile',
375 ]
376