[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: |
Fri, 25 Sep 2020 09:26:07 -0400 |
On Fri, Sep 25, 2020 at 03:15:57PM +0200, Markus Armbruster wrote:
> Cleber Rosa <crosa@redhat.com> writes:
>
> > On Wed, Sep 23, 2020 at 02:37:27PM -0400, John Snow wrote:
> >> On 9/23/20 11:26 AM, Eduardo Habkost wrote:
> >> > 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, ...).
>
> This code:
>
> fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666)
> f = open(fd, 'r+', encoding='utf-8')
>
> >> Not known to me. It was introduced in 907b846653 as part of an effort to
> >> reduce rebuild times. Maybe this avoids a modification time change if the
> >> file already exists?
> >>
> >> Markus?
> >
> > AFACIT the change on 907b846653 is effective because of the "is new
> > text different from old text?" conditional. I can not see how the
> > separate/duplicate open/fdopen would contribute to that.
> >
> > But, let's hear from Markus.
>
> This was my best attempt to open the file read/write, creating it if it
> doesn't exist.
>
> Plain
>
> f = open(pathname, "r+", encoding='utf-8')
>
> fails instead of creates, and
>
> f = open(pathname, "w+", encoding='utf-8')
>
> truncates.
>
> If you know a better way, tell me!
Thanks for the explanation!
Yeah, it looks like there's no combination of open() flags that
would get translated to O_RDWR|O_CREAT.
Using os.open() like you did seems more straightforward than
catching FileNotFoundError.
--
Eduardo
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, (continued)
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Eduardo Habkost, 2020/09/23
- 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 <=
- Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, John Snow, 2020/09/25
Re: [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, Cleber Rosa, 2020/09/24
[PATCH v2 29/38] qapi/gen.py: delint with pylint, John Snow, 2020/09/22
[PATCH v2 10/38] qapi/common.py: delint with pylint, John Snow, 2020/09/22