[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic
From: |
Eduardo Habkost |
Subject: |
Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic |
Date: |
Wed, 23 Sep 2020 11:26:24 -0400 |
On Tue, Sep 22, 2020 at 05:00:51PM -0400, John Snow wrote:
> 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>
I really miss a comment below explaining why we use
open(os.open(pathname, ...), ...) instead of open(pathname, ...).
> ---
> scripts/qapi/gen.py | 24 ++++++++++--------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index ba32f776e6..cf340e66d4 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 Dict, Generator, List, Optional, Tuple
> @@ -64,21 +63,18 @@ 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)
> +
> 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:
> --
> 2.26.2
>
--
Eduardo
- Re: [PATCH v2 22/38] qapi/source.py: add type hint annotations, (continued)
- [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, John Snow, 2020/09/22
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic,
Eduardo Habkost <=
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, John Snow, 2020/09/23
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Cleber Rosa, 2020/09/24
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Markus Armbruster, 2020/09/25
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Daniel P . Berrangé, 2020/09/25
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Eric Blake, 2020/09/25
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Markus Armbruster, 2020/09/25
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Eric Blake, 2020/09/25
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Markus Armbruster, 2020/09/28
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, John Snow, 2020/09/28
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Eduardo Habkost, 2020/09/25