launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #27376
[Merge] ~cjwatson/launchpad:deconfuse-isort into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:deconfuse-isort into launchpad:master.
Commit message:
Adjust various imports to confuse isort a little less
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/406753
Move non-circular imports to their usual place near the top of the file. Avoid `cimport` as a local variable because `isort` gets that confused with the Cython keyword. Add `# isort: split` in a few cases where we need to stop `isort` moving things around.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:deconfuse-isort into launchpad:master.
diff --git a/lib/contrib/glock.py b/lib/contrib/glock.py
index de39d12..69def87 100644
--- a/lib/contrib/glock.py
+++ b/lib/contrib/glock.py
@@ -29,7 +29,6 @@ __doc__ += '\n@author: %s (U{%s})\n@version: %s' % (__author__[0],
__author__[1], __version__)
__all__ = ['GlobalLock', 'GlobalLockError', 'LockAlreadyAcquired', 'NotOwner']
-# Imports:
import sys, string, os, errno, re
# System-dependent imports for locking implementation:
diff --git a/lib/lp/app/browser/tests/test_vocabulary.py b/lib/lp/app/browser/tests/test_vocabulary.py
index 2904d8c..7265281 100644
--- a/lib/lp/app/browser/tests/test_vocabulary.py
+++ b/lib/lp/app/browser/tests/test_vocabulary.py
@@ -328,9 +328,7 @@ class TestProductPickerEntrySourceAdapter(TestCaseWithFactory):
def test_provides_commercial_subscription_expired(self):
product = self.factory.makeProduct(name='fnord')
self.factory.makeCommercialSubscription(product)
- import datetime
- import pytz
- then = datetime.datetime(2005, 6, 15, 0, 0, 0, 0, pytz.UTC)
+ then = datetime(2005, 6, 15, 0, 0, 0, 0, pytz.UTC)
with celebrity_logged_in('admin'):
product.commercial_subscription.date_expires = then
self.assertEqual(
diff --git a/lib/lp/code/browser/tests/test_codeimport.py b/lib/lp/code/browser/tests/test_codeimport.py
index f8b1470..0dd76c6 100644
--- a/lib/lp/code/browser/tests/test_codeimport.py
+++ b/lib/lp/code/browser/tests/test_codeimport.py
@@ -94,48 +94,48 @@ class TestImportDetails(TestCaseWithFactory):
def test_other_users_are_forbidden_to_change_codeimport(self):
# Unauthorized users are forbidden to edit an import.
- cimport = self.factory.makeCodeImport()
+ code_import = self.factory.makeCodeImport()
another_person = self.factory.makePerson()
with person_logged_in(another_person):
self.assertRaises(
- Unauthorized, create_initialized_view, cimport.branch,
+ Unauthorized, create_initialized_view, code_import.branch,
'+edit-import')
def test_branch_owner_of_import_can_edit_it(self):
# Owners are allowed to edit code import.
- cimport = self.factory.makeCodeImport()
- with person_logged_in(cimport.branch.owner):
+ code_import = self.factory.makeCodeImport()
+ with person_logged_in(code_import.branch.owner):
view = create_initialized_view(
- cimport.branch, '+edit-import', form={
+ code_import.branch, '+edit-import', form={
"field.actions.update": "update",
"field.url": "http://foo.test";
})
self.assertEqual([], view.errors)
- self.assertEqual('http://foo.test', cimport.url)
+ self.assertEqual('http://foo.test', code_import.url)
def test_branch_owner_of_import_cannot_change_status(self):
# Owners are allowed to edit code import.
- cimport = self.factory.makeCodeImport()
- original_url = cimport.url
- with person_logged_in(cimport.branch.owner):
+ code_import = self.factory.makeCodeImport()
+ original_url = code_import.url
+ with person_logged_in(code_import.branch.owner):
view = create_initialized_view(
- cimport.branch, '+edit-import', form={
+ code_import.branch, '+edit-import', form={
"field.actions.suspend": "Suspend",
"field.url": "http://foo.test";
})
self.assertEqual([], view.errors)
- self.assertEqual(original_url, cimport.url)
+ self.assertEqual(original_url, code_import.url)
def test_admin_can_change_code_import_status(self):
# Owners are allowed to edit code import.
- cimport = self.factory.makeCodeImport()
+ code_import = self.factory.makeCodeImport()
with admin_logged_in():
view = create_initialized_view(
- cimport.branch, '+edit-import', form={
+ code_import.branch, '+edit-import', form={
"field.actions.suspend": "Suspend",
"field.url": "http://foo.test";
})
self.assertEqual([], view.errors)
- self.assertEqual("http://foo.test";, cimport.url)
+ self.assertEqual("http://foo.test";, code_import.url)
self.assertEqual(
- CodeImportReviewStatus.SUSPENDED, cimport.review_status)
+ CodeImportReviewStatus.SUSPENDED, code_import.review_status)
diff --git a/lib/lp/code/bzr.py b/lib/lp/code/bzr.py
index e823fc2..8e2e264 100644
--- a/lib/lp/code/bzr.py
+++ b/lib/lp/code/bzr.py
@@ -19,7 +19,7 @@ __all__ = [
# FIRST Ensure correct plugins are loaded. Do not delete this comment or the
# line below this comment.
-import lp.codehosting # noqa: F401
+import lp.codehosting # noqa: F401 # isort: split
from breezy.branch import UnstackableBranchFormat
from breezy.bzr.branch import (
diff --git a/lib/lp/code/interfaces/branch.py b/lib/lp/code/interfaces/branch.py
index 45c8086..b8d52e5 100644
--- a/lib/lp/code/interfaces/branch.py
+++ b/lib/lp/code/interfaces/branch.py
@@ -50,6 +50,7 @@ from lazr.restful.fields import (
ReferenceChoice,
)
from lazr.restful.interface import copy_field
+from lazr.uri import URI
from six.moves import http_client
from zope.component import getUtility
from zope.interface import (
@@ -157,7 +158,6 @@ class BranchURIField(URIField):
def _validate(self, value):
# import here to avoid circular import
from lp.services.webapp import canonical_url
- from lazr.uri import URI
# Can't use super-- this derives from an old-style class
URIField._validate(self, value)
diff --git a/lib/lp/code/interfaces/webservice.py b/lib/lp/code/interfaces/webservice.py
index 428c998..02546e9 100644
--- a/lib/lp/code/interfaces/webservice.py
+++ b/lib/lp/code/interfaces/webservice.py
@@ -35,6 +35,9 @@ __all__ = [
'ISourcePackageRecipeBuild',
]
+# XXX: JonathanLange 2010-11-09 bug=673083: Legacy work-around for circular
+# import bugs. Break this up into a per-package thing.
+from lp import _schema_circular_imports # noqa: F401
# The exceptions are imported so that they can produce the special
# status code defined by error_status when they are raised.
from lp.code.errors import (
@@ -70,9 +73,3 @@ from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipe
from lp.code.interfaces.sourcepackagerecipebuild import (
ISourcePackageRecipeBuild,
)
-
-
-# XXX: JonathanLange 2010-11-09 bug=673083: Legacy work-around for circular
-# import bugs. Break this up into a per-package thing.
-from lp import _schema_circular_imports
-_schema_circular_imports
diff --git a/lib/lp/codehosting/puller/worker.py b/lib/lp/codehosting/puller/worker.py
index 4ccfcbe..e3c23a6 100644
--- a/lib/lp/codehosting/puller/worker.py
+++ b/lib/lp/codehosting/puller/worker.py
@@ -8,7 +8,7 @@ import sys
# FIRST Ensure correct plugins are loaded. Do not delete this comment or the
# line below this comment.
-import lp.codehosting # noqa: F401
+import lp.codehosting # noqa: F401 # isort: split
from breezy import (
errors,
diff --git a/lib/lp/registry/tests/test_prf_filter.py b/lib/lp/registry/tests/test_prf_filter.py
index 1dc0d24..093dc79 100644
--- a/lib/lp/registry/tests/test_prf_filter.py
+++ b/lib/lp/registry/tests/test_prf_filter.py
@@ -3,24 +3,24 @@
"""Tests for lp.registry.scripts.productreleasefinder.filter."""
+import logging
import unittest
+from lp.registry.scripts.productreleasefinder.filter import (
+ Filter,
+ FilterPattern,
+ )
+
class Filter_Logging(unittest.TestCase):
def testCreatesDefaultLogger(self):
"""Filter creates a default logger."""
- from lp.registry.scripts.productreleasefinder.filter import (
- Filter)
- from logging import Logger
f = Filter()
- self.assertTrue(isinstance(f.log, Logger))
+ self.assertTrue(isinstance(f.log, logging.Logger))
def testCreatesChildLogger(self):
"""Filter creates a child logger if given a parent."""
- from lp.registry.scripts.productreleasefinder.filter import (
- Filter)
- from logging import getLogger
- parent = getLogger("foo")
+ parent = logging.getLogger("foo")
f = Filter(log_parent=parent)
self.assertEqual(f.log.parent, parent)
@@ -28,15 +28,11 @@ class Filter_Logging(unittest.TestCase):
class Filter_Init(unittest.TestCase):
def testDefaultFiltersProperty(self):
"""Filter constructor initializes filters property to empty dict."""
- from lp.registry.scripts.productreleasefinder.filter import (
- Filter)
f = Filter()
self.assertEqual(f.filters, [])
def testFiltersPropertyGiven(self):
"""Filter constructor accepts argument to set filters property."""
- from lp.registry.scripts.productreleasefinder.filter import (
- Filter)
f = Filter(["wibble"])
self.assertEqual(len(f.filters), 1)
self.assertEqual(f.filters[0], "wibble")
@@ -45,14 +41,10 @@ class Filter_Init(unittest.TestCase):
class Filter_CheckUrl(unittest.TestCase):
def testNoFilters(self):
"""Filter.check returns None if there are no filters."""
- from lp.registry.scripts.productreleasefinder.filter import (
- Filter)
f = Filter()
self.assertEqual(f.check("file:///subdir/file"), None)
def makeFilter(self, key, urlglob):
- from lp.registry.scripts.productreleasefinder.filter import (
- Filter, FilterPattern)
pattern = FilterPattern(key, urlglob)
return Filter([pattern])
@@ -85,8 +77,6 @@ class Filter_CheckUrl(unittest.TestCase):
class Filter_IsPossibleParentUrl(unittest.TestCase):
def makeFilter(self, key, urlglob):
- from lp.registry.scripts.productreleasefinder.filter import (
- Filter, FilterPattern)
pattern = FilterPattern(key, urlglob)
return Filter([pattern])
diff --git a/lib/lp/registry/tests/test_prf_hose.py b/lib/lp/registry/tests/test_prf_hose.py
index c679cc8..f56eb57 100644
--- a/lib/lp/registry/tests/test_prf_hose.py
+++ b/lib/lp/registry/tests/test_prf_hose.py
@@ -3,11 +3,17 @@
"""Tests for lp.registry.scripts.productreleasefinder.hose."""
+import logging
import os
import shutil
import tempfile
import unittest
+from lp.registry.scripts.productreleasefinder.filter import (
+ Filter,
+ FilterPattern,
+ )
+from lp.registry.scripts.productreleasefinder.hose import Hose
from lp.testing import reset_logging
@@ -68,16 +74,12 @@ class InstrumentedMethodObserver:
class Hose_Logging(unittest.TestCase):
def testCreatesDefaultLogger(self):
"""Hose creates a default logger."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
- from logging import Logger
h = Hose()
- self.assertTrue(isinstance(h.log, Logger))
+ self.assertTrue(isinstance(h.log, logging.Logger))
def testCreatesChildLogger(self):
"""Hose creates a child logger if given a parent."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
- from logging import getLogger
- parent = getLogger("foo")
+ parent = logging.getLogger("foo")
h = Hose(log_parent=parent)
self.assertEqual(h.log.parent, parent)
@@ -85,23 +87,16 @@ class Hose_Logging(unittest.TestCase):
class Hose_Filter(unittest.TestCase):
def testCreatesFilterObject(self):
"""Hose creates a Filter object."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
- from lp.registry.scripts.productreleasefinder.filter import (
- Filter)
h = Hose()
self.assertTrue(isinstance(h.filter, Filter))
def testDefaultsFiltersToEmptyDict(self):
"""Hose creates Filter object with empty dictionary."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
h = Hose()
self.assertEqual(h.filter.filters, [])
def testCreatesFiltersWithGiven(self):
"""Hose creates Filter object with dictionary given."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
- from lp.registry.scripts.productreleasefinder.filter import (
- FilterPattern)
pattern = FilterPattern("foo", "http:e*")
h = Hose([pattern])
self.assertEqual(len(h.filter.filters), 1)
@@ -111,7 +106,6 @@ class Hose_Filter(unittest.TestCase):
class Hose_Urls(unittest.TestCase):
def testCallsReduceWork(self):
"""Hose constructor calls reduceWork function."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
h = Hose.__new__(Hose)
class Observer(InstrumentedMethodObserver):
@@ -128,9 +122,6 @@ class Hose_Urls(unittest.TestCase):
def testPassesUrlList(self):
"""Hose constructor passes url list to reduceWork."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
- from lp.registry.scripts.productreleasefinder.filter import (
- FilterPattern)
pattern = FilterPattern("foo", "http://archive.ubuntu.com/e*";)
h = Hose.__new__(Hose)
@@ -148,8 +139,6 @@ class Hose_Urls(unittest.TestCase):
def testSetsUrlProperty(self):
"""Hose constructor sets urls property to reduceWork return value."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
-
class TestHose(Hose):
def reduceWork(self, url_list):
return "wibble"
@@ -161,20 +150,17 @@ class Hose_Urls(unittest.TestCase):
class Hose_ReduceWork(unittest.TestCase):
def testEmptyList(self):
"""Hose.reduceWork returns empty list when given one."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
h = Hose()
self.assertEqual(h.reduceWork([]), [])
def testReducedList(self):
"""Hose.reduceWork returns same list when nothing to do."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
h = Hose()
self.assertEqual(h.reduceWork(["http://localhost/";, "file:///usr/"]),
["http://localhost/";, "file:///usr/"])
def testReducesList(self):
"""Hose.reduceWork removes children elements from list."""
- from lp.registry.scripts.productreleasefinder.hose import Hose
h = Hose()
self.assertEqual(h.reduceWork(["http://localhost/";,
"http://localhost/foo/bar/";,
@@ -219,9 +205,6 @@ class Hose_LimitWalk(unittest.TestCase):
fp.close()
# Run the hose over the test data
- from lp.registry.scripts.productreleasefinder.hose import Hose
- from lp.registry.scripts.productreleasefinder.filter import (
- FilterPattern)
pattern = FilterPattern("key", self.release_url +
"/foo/1.*/source/foo-1.*.tar.gz")
hose = Hose([pattern])
diff --git a/lib/lp/registry/tests/test_prf_log.py b/lib/lp/registry/tests/test_prf_log.py
index d55f54f..2d663a0 100644
--- a/lib/lp/registry/tests/test_prf_log.py
+++ b/lib/lp/registry/tests/test_prf_log.py
@@ -7,38 +7,32 @@
__author__ = "Scott James Remnant <scott@xxxxxxxxxxxxx>"
+import logging
import unittest
+from lp.registry.scripts.productreleasefinder.log import get_logger
+
class GetLogger(unittest.TestCase):
def testLogger(self):
"""get_logger returns a Logger instance."""
- from lp.registry.scripts.productreleasefinder.log import get_logger
- from logging import Logger
- self.assertTrue(isinstance(get_logger("test"), Logger))
+ self.assertTrue(isinstance(get_logger("test"), logging.Logger))
def testNoParent(self):
"""get_logger works if no parent is given."""
- from lp.registry.scripts.productreleasefinder.log import get_logger
self.assertEqual(get_logger("test").name, "test")
def testRootParent(self):
"""get_logger works if root logger is given."""
- from lp.registry.scripts.productreleasefinder.log import get_logger
- from logging import root
- self.assertEqual(get_logger("test", root).name, "test")
+ self.assertEqual(get_logger("test", logging.root).name, "test")
def testNormalParent(self):
"""get_logger works if non-root logger is given."""
- from lp.registry.scripts.productreleasefinder.log import get_logger
- from logging import getLogger
- parent = getLogger("foo")
+ parent = logging.getLogger("foo")
self.assertEqual(get_logger("test", parent).name, "foo.test")
def testDeepParent(self):
"""get_logger works if deep-level logger is given."""
- from lp.registry.scripts.productreleasefinder.log import get_logger
- from logging import getLogger
- getLogger("foo")
- parent2 = getLogger("foo.bar")
+ logging.getLogger("foo")
+ parent2 = logging.getLogger("foo.bar")
self.assertEqual(get_logger("test", parent2).name, "foo.bar.test")
diff --git a/lib/lp/services/librarian/client.py b/lib/lp/services/librarian/client.py
index 6787768..a26e39f 100644
--- a/lib/lp/services/librarian/client.py
+++ b/lib/lp/services/librarian/client.py
@@ -37,6 +37,7 @@ from six.moves.urllib.parse import (
urlunparse,
)
from six.moves.urllib.request import urlopen
+from sqlobject import SQLObjectNotFound
from storm.store import Store
from zope.interface import implementer
@@ -385,7 +386,7 @@ class FileDownloadClient:
inaccessible.
"""
from lp.services.librarian.model import LibraryFileAlias
- from sqlobject import SQLObjectNotFound
+
try:
lfa = LibraryFileAlias.get(aliasID)
except SQLObjectNotFound:
diff --git a/lib/lp/services/mail/notificationrecipientset.py b/lib/lp/services/mail/notificationrecipientset.py
index 7297da2..6004399 100644
--- a/lib/lp/services/mail/notificationrecipientset.py
+++ b/lib/lp/services/mail/notificationrecipientset.py
@@ -14,7 +14,10 @@ from operator import attrgetter
import six
from zope.interface import implementer
-from zope.security.proxy import isinstance as zope_isinstance
+from zope.security.proxy import (
+ isinstance as zope_isinstance,
+ removeSecurityProxy,
+ )
from lp.registry.interfaces.person import IPerson
from lp.services.mail.interfaces import (
@@ -107,8 +110,8 @@ class NotificationRecipientSet:
def add(self, persons, reason, header):
"""See `INotificationRecipientSet`."""
- from zope.security.proxy import removeSecurityProxy
from lp.registry.model.person import get_recipients
+
if (IPerson.providedBy(persons) or
zope_isinstance(persons, StubPerson)):
persons = [persons]
@@ -143,8 +146,8 @@ class NotificationRecipientSet:
def remove(self, persons):
"""See `INotificationRecipientSet`."""
- from zope.security.proxy import removeSecurityProxy
from lp.registry.model.person import get_recipients
+
if IPerson.providedBy(persons):
persons = [persons]
for person in persons:
diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
index 2d07f26..f6e931b 100644
--- a/lib/lp/testing/layers.py
+++ b/lib/lp/testing/layers.py
@@ -78,6 +78,7 @@ from six.moves.urllib.error import (
)
from six.moves.urllib.parse import urlparse
from six.moves.urllib.request import urlopen
+from storm.uri import URI
from talisker.context import Context
import transaction
from webob.request import environ_from_url as orig_environ_from_url
@@ -935,9 +936,7 @@ class LaunchpadLayer(LibrarianLayer, MemcachedLayer, RabbitMQLayer):
in the testSetUp().
"""
if LaunchpadLayer._raw_sessiondb_connection is None:
- from storm.uri import URI
- from lp.services.webapp.adapter import (
- LaunchpadSessionDatabase)
+ from lp.services.webapp.adapter import LaunchpadSessionDatabase
launchpad_session_database = LaunchpadSessionDatabase(
URI('launchpad-session:'))
LaunchpadLayer._raw_sessiondb_connection = (
diff --git a/lib/lp/translations/scripts/translations_to_branch.py b/lib/lp/translations/scripts/translations_to_branch.py
index 0e57a88..f07abd5 100644
--- a/lib/lp/translations/scripts/translations_to_branch.py
+++ b/lib/lp/translations/scripts/translations_to_branch.py
@@ -15,7 +15,7 @@ import os.path
# FIRST Ensure correct plugins are loaded. Do not delete this comment or the
# line below this comment.
-import lp.codehosting # noqa: F401
+import lp.codehosting # noqa: F401 # isort: split
from breezy.errors import NotBranchError
from breezy.revision import NULL_REVISION
diff --git a/lib/sqlobject/__init__.py b/lib/sqlobject/__init__.py
index 3e5f9e4..5450477 100644
--- a/lib/sqlobject/__init__.py
+++ b/lib/sqlobject/__init__.py
@@ -7,13 +7,14 @@ __metaclass__ = type
# SKIP this file when reformatting, due to the sys mangling.
import datetime
+import sys
import six
from storm.expr import SQL
from storm.sqlobject import * # noqa: F401,F403
+
# Provide the same interface from these other locations.
-import sys
sys.modules['sqlobject.joins'] = sys.modules['sqlobject']
sys.modules['sqlobject.sqlbuilder'] = sys.modules['sqlobject']
del sys
