← Back to team overview

launchpad-reviewers team mailing list archive

[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():