From 9c65bf254e95df1755db017de89db15b8b221840 Mon Sep 17 00:00:00 2001 From: Paul Eggleton Date: Tue, 15 Jan 2019 11:35:25 +1300 Subject: [PATCH] Use try...finally or with to ensure files get closed Best practices state that you should use a mechanism that ensures files get closed in case of any error, so let's do that. Signed-off-by: Paul Eggleton --- layerindex/bulkchange.py | 18 ++++++++----- layerindex/views.py | 41 ++++++++++++++++------------- rrs/tools/rrs_distros.py | 16 +++++------ rrs/tools/rrs_maintainer_history.py | 37 +++++++++++++------------- 4 files changed, 59 insertions(+), 53 deletions(-) diff --git a/layerindex/bulkchange.py b/layerindex/bulkchange.py index 2e7f6e8..f6506ef 100644 --- a/layerindex/bulkchange.py +++ b/layerindex/bulkchange.py @@ -55,17 +55,21 @@ def generate_patches(tinfoil, fetchdir, changeset, outputdir): (tmptarfd, tmptarname) = tempfile.mkstemp('.tar.gz', 'bulkchange-', outputdir) tmptarfile = os.fdopen(tmptarfd, "wb") tar = tarfile.open(None, "w:gz", tmptarfile) - for patch in patches: - patchfn = os.path.join(tmpoutdir, patch) - tar.add(patchfn, arcname=patch) - tar.close() + try: + for patch in patches: + patchfn = os.path.join(tmpoutdir, patch) + tar.add(patchfn, arcname=patch) + finally: + tar.close() ret = tmptarname elif len(patches) == 1: (tmppatchfd, tmppatchname) = tempfile.mkstemp('.patch', 'bulkchange-', outputdir) tmppatchfile = os.fdopen(tmppatchfd, "wb") - with open(os.path.join(tmpoutdir, patches[0]), "rb") as patchfile: - shutil.copyfileobj(patchfile, tmppatchfile) - tmppatchfile.close() + try: + with open(os.path.join(tmpoutdir, patches[0]), "rb") as patchfile: + shutil.copyfileobj(patchfile, tmppatchfile) + finally: + tmppatchfile.close() ret = tmppatchname shutil.rmtree(tmpoutdir) diff --git a/layerindex/views.py b/layerindex/views.py index 6c7a919..adb0774 100644 --- a/layerindex/views.py +++ b/layerindex/views.py @@ -1457,25 +1457,28 @@ def task_log_view(request, task_id): f = open(os.path.join(settings.TASK_LOG_DIR, 'task_%s.log' % task_id), 'rb') except FileNotFoundError: raise Http404 - f.seek(int(start)) - # We need to escape this or else things that look like tags in the output - # will be interpreted as such by the browser - data = escape(f.read()) - response = HttpResponse(data) - if result.ready(): - response['Task-Done'] = '1' - updateobj = get_object_or_404(Update, task_id=task_id) - response['Task-Duration'] = utils.timesince2(updateobj.started, updateobj.finished) - response['Task-Progress'] = 100 - if result.info: - if isinstance(result.info, dict): - response['Task-Result'] = result.info.get('retcode', None) - else: - response['Task-Result'] = -1 - else: - response['Task-Done'] = '0' - preader = utils.ProgressReader(settings.TASK_LOG_DIR, task_id) - response['Task-Progress'] = preader.read() + try: + f.seek(int(start)) + # We need to escape this or else things that look like tags in the output + # will be interpreted as such by the browser + data = escape(f.read()) + response = HttpResponse(data) + if result.ready(): + response['Task-Done'] = '1' + updateobj = get_object_or_404(Update, task_id=task_id) + response['Task-Duration'] = utils.timesince2(updateobj.started, updateobj.finished) + response['Task-Progress'] = 100 + if result.info: + if isinstance(result.info, dict): + response['Task-Result'] = result.info.get('retcode', None) + else: + response['Task-Result'] = -1 + else: + response['Task-Done'] = '0' + preader = utils.ProgressReader(settings.TASK_LOG_DIR, task_id) + response['Task-Progress'] = preader.read() + finally: + f.close() return response def task_stop_view(request, task_id): diff --git a/rrs/tools/rrs_distros.py b/rrs/tools/rrs_distros.py index 41818b4..e803328 100755 --- a/rrs/tools/rrs_distros.py +++ b/rrs/tools/rrs_distros.py @@ -69,15 +69,13 @@ def search_package_in_distros(pkglst_dir, recipe, data): else: pn = recipe_name - f = open(os.path.join(pkglst_dir, distro_file), "r") - for line in f: - (pkg, section) = line.split(":") - if pn == pkg: - distro_complete = distro + "-" + section[:-1] - distros[distro_complete] = pn - f.close() - break - f.close() + with open(os.path.join(pkglst_dir, distro_file), "r") as f: + for line in f: + (pkg, section) = line.split(":") + if pn == pkg: + distro_complete = distro + "-" + section[:-1] + distros[distro_complete] = pn + break return distros diff --git a/rrs/tools/rrs_maintainer_history.py b/rrs/tools/rrs_maintainer_history.py index c4beaf1..b38fe44 100755 --- a/rrs/tools/rrs_maintainer_history.py +++ b/rrs/tools/rrs_maintainer_history.py @@ -102,27 +102,28 @@ def maintainers_inc_history(options, logger, maintplan, layerbranch, repodir, la utils.runcmd("git checkout %s -f" % commit, repodir, logger=logger) - lines = [line.strip() for line in open(maintainers_full_path)] - for line in lines: - res = get_recipe_maintainer(line, logger) - if res: - (pn, name, email) = res - qry = Recipe.objects.filter(pn = pn, layerbranch = layerbranch) + with open(maintainers_full_path, 'r') as f: + for line in f: + line = line.strip() + res = get_recipe_maintainer(line, logger) + if res: + (pn, name, email) = res + qry = Recipe.objects.filter(pn = pn, layerbranch = layerbranch) - if qry: - m = Maintainer.create_or_update(name, email) + if qry: + m = Maintainer.create_or_update(name, email) - rm = RecipeMaintainer() - rm.recipe = qry[0] - rm.maintainer = m - rm.history = rms - rm.save() + rm = RecipeMaintainer() + rm.recipe = qry[0] + rm.maintainer = m + rm.history = rms + rm.save() - logger.debug("%s: Change maintainer to %s in commit %s." % \ - (pn, m.name, commit)) - else: - logger.debug("%s: Not found in %s." % \ - (pn, layerbranch)) + logger.debug("%s: Change maintainer to %s in commit %s." % \ + (pn, m.name, commit)) + else: + logger.debug("%s: Not found in %s." % \ + (pn, layerbranch)) # set missing recipes to no maintainer for recipe in layerbranch.recipe_set.all():