launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25057
[Merge] ~cjwatson/launchpad:more-context-managers into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:more-context-managers into launchpad:master.
Commit message:
Use context managers instead of open().{read,write}()
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/387723
This fixes ResourceWarnings on Python 3.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:more-context-managers into launchpad:master.
diff --git a/database/schema/upgrade.py b/database/schema/upgrade.py
index dd0ec18..fa536ef 100755
--- a/database/schema/upgrade.py
+++ b/database/schema/upgrade.py
@@ -221,7 +221,8 @@ def apply_other(con, script, no_commit=False):
log.info("Applying %s" % script)
cur = con.cursor()
path = os.path.join(os.path.dirname(__file__), script)
- sql = open(path).read()
+ with open(path) as f:
+ sql = f.read()
if not sql.rstrip().endswith(';'):
# This is important because patches are concatenated together
# into a single script when we apply them to a replicated
diff --git a/lib/lp/archiveuploader/dscfile.py b/lib/lp/archiveuploader/dscfile.py
index fbe1e25..4d88239 100644
--- a/lib/lp/archiveuploader/dscfile.py
+++ b/lib/lp/archiveuploader/dscfile.py
@@ -803,7 +803,8 @@ def find_copyright(source_dir, logger):
raise UploadWarning("No copyright file found.")
logger.debug("Copying copyright contents.")
- return open(copyright_file).read().strip()
+ with open(copyright_file) as f:
+ return f.read().strip()
def find_changelog(source_dir, logger):
@@ -824,7 +825,8 @@ def find_changelog(source_dir, logger):
# Move the changelog file out of the package direcotry
logger.debug("Found changelog")
- return open(changelog_file, 'r').read()
+ with open(changelog_file) as changelog:
+ return changelog.read()
def check_format_1_0_files(filename, file_type_counts,
diff --git a/lib/lp/bugs/scripts/cveimport.py b/lib/lp/bugs/scripts/cveimport.py
index f1f41b5..b202974 100644
--- a/lib/lp/bugs/scripts/cveimport.py
+++ b/lib/lp/bugs/scripts/cveimport.py
@@ -204,7 +204,8 @@ class CVEUpdater(LaunchpadCronScript):
self.logger.info('Initializing...')
if self.options.cvefile is not None:
try:
- cve_db = open(self.options.cvefile, 'r').read()
+ with open(self.options.cvefile) as f:
+ cve_db = f.read()
except IOError:
raise LaunchpadScriptFailure(
'Unable to open CVE database in %s'
diff --git a/lib/lp/bugs/tests/externalbugtracker.py b/lib/lp/bugs/tests/externalbugtracker.py
index afb1d81..8d3dfe9 100644
--- a/lib/lp/bugs/tests/externalbugtracker.py
+++ b/lib/lp/bugs/tests/externalbugtracker.py
@@ -1666,7 +1666,8 @@ class TestDebBugsDB:
raise debbugs.LogParseFailed(
'debbugs-log.pl exited with code 512')
- comment_data = open(self.data_file).read()
+ with open(self.data_file) as f:
+ comment_data = f.read()
bug._emails = []
bug.comments = [comment.strip() for comment in
comment_data.split('--\n')]
diff --git a/lib/lp/buildmaster/tests/harness.py b/lib/lp/buildmaster/tests/harness.py
index 5ffc75f..0cb68df 100644
--- a/lib/lp/buildmaster/tests/harness.py
+++ b/lib/lp/buildmaster/tests/harness.py
@@ -40,7 +40,8 @@ class BuilddManagerTestSetup(TacTestSetup):
remove_tree(self.root)
os.makedirs(self.root)
if self.logfilecontent is not None:
- open(self.logfile, "w").write(self.logfilecontent)
+ with open(self.logfile, "w") as f:
+ f.write(self.logfilecontent)
@property
def root(self):
diff --git a/lib/lp/codehosting/codeimport/tests/test_dispatcher.py b/lib/lp/codehosting/codeimport/tests/test_dispatcher.py
index 004add2..0f18ef7 100644
--- a/lib/lp/codehosting/codeimport/tests/test_dispatcher.py
+++ b/lib/lp/codehosting/codeimport/tests/test_dispatcher.py
@@ -112,7 +112,8 @@ class TestCodeImportDispatcherUnit(TestCase):
dispatcher.worker_script = script_path
proc = dispatcher.dispatchJob(10)
proc.wait()
- arglist = self.filterOutLoggingOptions(eval(open(output_path).read()))
+ with open(output_path) as f:
+ arglist = self.filterOutLoggingOptions(eval(f.read()))
self.assertEqual(['10'], arglist)
def test_findAndDispatchJob_jobWaiting(self):
diff --git a/lib/lp/services/config/tests/test_fixture.py b/lib/lp/services/config/tests/test_fixture.py
index 045ad06..7db44b4 100644
--- a/lib/lp/services/config/tests/test_fixture.py
+++ b/lib/lp/services/config/tests/test_fixture.py
@@ -40,11 +40,14 @@ class TestConfigFixture(TestCase):
for base in to_copy:
path = 'configs/testtestconfig/' + base
source = 'configs/testrunner/' + base
- old = open(source, 'rb').read()
- new = open(path, 'rb').read()
+ with open(source, 'rb') as f:
+ old = f.read()
+ with open(path, 'rb') as f:
+ new = f.read()
self.assertEqual(old, new)
confpath = 'configs/testtestconfig/launchpad-lazr.conf'
- lazr_config = open(confpath, 'rb').read()
+ with open(confpath) as f:
+ lazr_config = f.read()
self.assertEqual(
"[meta]\n"
"extends: ../testrunner/launchpad-lazr.conf",
diff --git a/lib/lp/services/mail/tests/helpers.py b/lib/lp/services/mail/tests/helpers.py
index e7afd33..600794f 100644
--- a/lib/lp/services/mail/tests/helpers.py
+++ b/lib/lp/services/mail/tests/helpers.py
@@ -20,5 +20,6 @@ def read_test_message(filename):
The test messages are located in lp/services/mail/tests/emails
"""
- message_string = open(os.path.join(testmails_path, filename)).read()
+ with open(os.path.join(testmails_path, filename)) as f:
+ message_string = f.read()
return signed_message_from_string(message_string)
diff --git a/lib/lp/services/osutils.py b/lib/lp/services/osutils.py
index 7eafc40..c670e62 100644
--- a/lib/lp/services/osutils.py
+++ b/lib/lp/services/osutils.py
@@ -154,7 +154,8 @@ def get_pid_from_file(pidfile_path):
if not os.path.exists(pidfile_path):
return None
# Get the pid.
- pid = open(pidfile_path, 'r').read().split()[0]
+ with open(pidfile_path) as pidfile:
+ pid = pidfile.read().split()[0]
try:
pid = int(pid)
except ValueError:
diff --git a/lib/lp/services/pidfile.py b/lib/lp/services/pidfile.py
index d5e2a2b..e8899b8 100644
--- a/lib/lp/services/pidfile.py
+++ b/lib/lp/services/pidfile.py
@@ -81,7 +81,8 @@ def get_pid(service_name, use_config=None):
"""
pidfile = pidfile_path(service_name, use_config)
try:
- pid = open(pidfile).read()
+ with open(pidfile) as f:
+ pid = f.read()
return int(pid)
except IOError:
return None
diff --git a/lib/lp/services/spriteutils.py b/lib/lp/services/spriteutils.py
index dd1d2de..b007a05 100644
--- a/lib/lp/services/spriteutils.py
+++ b/lib/lp/services/spriteutils.py
@@ -224,7 +224,8 @@ class SpriteUtil:
def loadPositioning(self, filename):
"""Load file with the positions of sprites in the combined image."""
- json = open(filename).read()
+ with open(filename) as f:
+ json = f.read()
# Remove comments from the beginning of the file.
start = json.index('{')
json = json[start:]
diff --git a/lib/lp/services/tests/test_osutils.py b/lib/lp/services/tests/test_osutils.py
index d415d02..8429c33 100644
--- a/lib/lp/services/tests/test_osutils.py
+++ b/lib/lp/services/tests/test_osutils.py
@@ -74,32 +74,31 @@ class TestOpenForWriting(TestCase):
# open_for_writing opens a file for, umm, writing.
directory = self.makeTemporaryDirectory()
filename = os.path.join(directory, 'foo')
- fp = open_for_writing(filename, 'w')
- fp.write("Hello world!\n")
- fp.close()
- self.assertEqual("Hello world!\n", open(filename).read())
+ with open_for_writing(filename, 'w') as fp:
+ fp.write("Hello world!\n")
+ with open(filename) as fp:
+ self.assertEqual("Hello world!\n", fp.read())
def test_opens_for_writing_append(self):
# open_for_writing can also open to append.
directory = self.makeTemporaryDirectory()
filename = os.path.join(directory, 'foo')
- fp = open_for_writing(filename, 'w')
- fp.write("Hello world!\n")
- fp.close()
- fp = open_for_writing(filename, 'a')
- fp.write("Next line\n")
- fp.close()
- self.assertEqual("Hello world!\nNext line\n", open(filename).read())
+ with open_for_writing(filename, 'w') as fp:
+ fp.write("Hello world!\n")
+ with open_for_writing(filename, 'a') as fp:
+ fp.write("Next line\n")
+ with open(filename) as fp:
+ self.assertEqual("Hello world!\nNext line\n", fp.read())
def test_even_if_directory_doesnt_exist(self):
# open_for_writing will open a file for writing even if the directory
# doesn't exist.
directory = self.makeTemporaryDirectory()
filename = os.path.join(directory, 'foo', 'bar', 'baz', 'filename')
- fp = open_for_writing(filename, 'w')
- fp.write("Hello world!\n")
- fp.close()
- self.assertEqual("Hello world!\n", open(filename).read())
+ with open_for_writing(filename, 'w') as fp:
+ fp.write("Hello world!\n")
+ with open(filename) as fp:
+ self.assertEqual("Hello world!\n", fp.read())
class TestWriteFile(TestCase):
diff --git a/lib/lp/services/webapp/metazcml.py b/lib/lp/services/webapp/metazcml.py
index 909fe2d..0f6b21c 100644
--- a/lib/lp/services/webapp/metazcml.py
+++ b/lib/lp/services/webapp/metazcml.py
@@ -419,7 +419,8 @@ def favicon(_context, for_, file):
@implementer(IFavicon)
class Favicon(FaviconRendererBase):
path = file
- data = open(file, 'rb').read()
+ with open(file, 'rb') as f:
+ data = f.read()
name = "favicon.ico"
permission = CheckerPublic
diff --git a/lib/lp/services/webapp/tests/test_sigusr2.py b/lib/lp/services/webapp/tests/test_sigusr2.py
index 8cd6845..87dfd4e 100644
--- a/lib/lp/services/webapp/tests/test_sigusr2.py
+++ b/lib/lp/services/webapp/tests/test_sigusr2.py
@@ -63,9 +63,10 @@ class SIGUSR2TestCase(unittest.TestCase):
# Confirm content in the main log and the cycled log are what we
# expect.
- self.assertEqual(
- open(cycled_log, 'r').read(), 'Message 1\nMessage 2\n')
- self.assertEqual(open(main_log, 'r').read(), 'Message 3\n')
+ with open(cycled_log) as f:
+ self.assertEqual(f.read(), 'Message 1\nMessage 2\n')
+ with open(main_log) as f:
+ self.assertEqual(f.read(), 'Message 3\n')
def sync(self, step):
retries = 200
diff --git a/lib/lp/soyuz/scripts/gina/packages.py b/lib/lp/soyuz/scripts/gina/packages.py
index 6e2f090..2b44a5a 100644
--- a/lib/lp/soyuz/scripts/gina/packages.py
+++ b/lib/lp/soyuz/scripts/gina/packages.py
@@ -123,12 +123,14 @@ def read_dsc(package, version, component, distro_name, archive_root):
distro_name, archive_root)
try:
- dsc = open(dsc_path).read().strip()
+ with open(dsc_path) as f:
+ dsc = f.read().strip()
fullpath = os.path.join(source_dir, "debian", "changelog")
changelog = None
if os.path.exists(fullpath):
- changelog = open(fullpath).read().strip()
+ with open(fullpath) as f:
+ changelog = f.read().strip()
else:
log.warn("No changelog file found for %s in %s" %
(package, source_dir))
@@ -139,7 +141,8 @@ def read_dsc(package, version, component, distro_name, archive_root):
for fullpath in glob.glob(globpath):
if not os.path.exists(fullpath):
continue
- copyright = open(fullpath).read().strip()
+ with open(fullpath) as f:
+ copyright = f.read().strip()
if copyright is None:
log.warn(
@@ -559,5 +562,6 @@ class BinaryPackageData(AbstractPackageData):
call("dpkg -e %s" % fullpath)
shlibfile = os.path.join("DEBIAN", "shlibs")
if os.path.exists(shlibfile):
- self.shlibs = open(shlibfile).read().strip()
+ with open(shlibfile) as f:
+ self.shlibs = f.read().strip()
log.debug("Grabbing shared library info from %s" % shlibfile)
diff --git a/lib/lp/testing/gpgkeys/__init__.py b/lib/lp/testing/gpgkeys/__init__.py
index f279eee..53a0845 100644
--- a/lib/lp/testing/gpgkeys/__init__.py
+++ b/lib/lp/testing/gpgkeys/__init__.py
@@ -94,7 +94,8 @@ def import_secret_test_key(keyfile='test@xxxxxxxxxxxxxxxxx'):
:param keyfile: The name of the file to be imported.
"""
gpghandler = getUtility(IGPGHandler)
- seckey = open(os.path.join(gpgkeysdir, keyfile)).read()
+ with open(os.path.join(gpgkeysdir, keyfile)) as f:
+ seckey = f.read()
return gpghandler.importSecretKey(seckey)
@@ -105,7 +106,8 @@ def test_pubkey_file_from_email(email_addr):
def test_pubkey_from_email(email_addr):
"""Get the on disk content for a test pubkey by email address."""
- return open(test_pubkey_file_from_email(email_addr)).read()
+ with open(test_pubkey_file_from_email(email_addr)) as f:
+ return f.read()
def test_keyrings():
diff --git a/lib/lp/testing/keyserver/web.py b/lib/lp/testing/keyserver/web.py
index faac2e8..86db0e5 100644
--- a/lib/lp/testing/keyserver/web.py
+++ b/lib/lp/testing/keyserver/web.py
@@ -145,7 +145,8 @@ class LookUp(Resource):
path = locate_key(self.root, filename)
if path is not None:
- content = cgi.escape(open(path).read())
+ with open(path) as f:
+ content = cgi.escape(f.read())
page = ('<html>\n<head>\n'
'<title>Results for Key %s</title>\n'
'</head>\n<body>'
diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
index fa89018..6cf747c 100644
--- a/lib/lp/testing/layers.py
+++ b/lib/lp/testing/layers.py
@@ -612,7 +612,8 @@ class MemcachedLayer(BaseLayer):
# Store the pidfile for other processes to kill.
pid_file = MemcachedLayer.getPidFile()
- open(pid_file, 'w').write(str(MemcachedLayer._memcached_process.pid))
+ with open(pid_file, 'w') as f:
+ f.write(str(MemcachedLayer._memcached_process.pid))
@classmethod
@profiled
diff --git a/utilities/list-pages b/utilities/list-pages
index bf08d0a..7bce9d8 100755
--- a/utilities/list-pages
+++ b/utilities/list-pages
@@ -121,7 +121,8 @@ def has_page_title(a):
def has_html_element(template):
- return '</html>' in open(template).read()
+ with open(template) as f:
+ return '</html>' in f.read()
def iter_page_adapters():