|
From: | John Snow |
Subject: | Re: [PATCH v5 30/36] qapi/gen.py: update write() to be more idiomatic |
Date: | Wed, 7 Oct 2020 12:25:48 -0400 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
On 10/7/20 8:32 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:Make the file handling here just a tiny bit more idiomatic. (I realize this is heavily subjective.) Use exist_ok=True for os.makedirs and remove the exception, use fdopen() to wrap the file descriptor in a File-like object, and use a context manager for managing the file pointer. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> --- scripts/qapi/gen.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 3624162bb77..579ee283297 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -14,7 +14,6 @@ # See the COPYING file in the top-level directory.from contextlib import contextmanager-import errno import os import re from typing import ( @@ -67,21 +66,19 @@ def write(self, output_dir: str) -> None: return pathname = os.path.join(output_dir, self.fname) odir = os.path.dirname(pathname) + if odir: - try: - os.makedirs(odir) - except os.error as e: - if e.errno != errno.EEXIST: - raise + os.makedirs(odir, exist_ok=True)I wouldn't call this part "heavily subjective". When wrote the old code, exist_ok was still off limits (it's new in 3.2).
It's cool if you agree, I just realize that what people consider idiomatic is subjective unless it's enforcable by a tool. This isn't.
"I made this look more like if I wrote it, which caused Dopamine" is a bad commit message. (But more true.)
+ + # use os.open for O_CREAT to create and read a non-existant file fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666) - f = open(fd, 'r+', encoding='utf-8') - text = self.get_content() - oldtext = f.read(len(text) + 1) - if text != oldtext: - f.seek(0) - f.truncate(0) - f.write(text) - f.close() + with os.fdopen(fd, 'r+', encoding='utf-8') as fp: + text = self.get_content() + oldtext = fp.read(len(text) + 1) + if text != oldtext: + fp.seek(0) + fp.truncate(0) + fp.write(text)def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str:Reviewed-by: Markus Armbruster <armbru@redhat.com>
Thanks, though :)
[Prev in Thread] | Current Thread | [Next in Thread] |