openstack team mailing list archive
-
openstack team
-
Mailing list archive
-
Message #16596
Re: One potential issue on the normalize_time() in timeutils
I create a patch for it https://review.openstack.org/#/c/12705/ and please help to review.
Thanks
--jyh
> -----Original Message-----
> From: openstack-bounces+yunhong.jiang=intel.com@xxxxxxxxxxxxxxxxxxx
> [mailto:openstack-bounces+yunhong.jiang=intel.com@xxxxxxxxxxxxxxxxxxx] On
> Behalf Of Jiang, Yunhong
> Sent: Monday, September 10, 2012 8:17 AM
> To: Robert Collins
> Cc: openstack@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Openstack] One potential issue on the normalize_time() in
> timeutils
>
> Rob, thanks for comments.
> I totally agree with you that aware datetime object is better than naive one.
> The key thing is, utcnow() will return naive object, that means in some time, we
> have to use naive object to compare with utcnow(), and we need a conversion
> function to convert from aware to naive. The normalize_time() is the best
> candidate for this purpose, but it will fail to convert to naive datetime object in
> some situation. That's why I send the mail. I just want to change the
> normalize_time() to make sure it will always return naive object.
>
> Thanks
> --jyh
>
> > -----Original Message-----
> > From: Robert Collins [mailto:robertc@xxxxxxxxxxxxxxxxx]
> > Sent: Monday, September 10, 2012 3:25 AM
> > To: Jiang, Yunhong
> > Cc: eglynn@xxxxxxxxxx; openstack@xxxxxxxxxxxxxxxxxxx
> > Subject: Re: [Openstack] One potential issue on the normalize_time()
> > in timeutils
> >
> > On Mon, Sep 10, 2012 at 3:09 AM, Jiang, Yunhong
> > <yunhong.jiang@xxxxxxxxx>
> > wrote:
> > > Hi, Eoghan and all,
> > >
> > > When I implement an enhancement to trusted_filter, I need
> > > utilize
> > timeutils() to parse ISO time. However, I suspect there is one
> > potential issue in
> > normalize_time() and want to get some input from your side.
> > >
> > > In normalize_time(), if the parameter "timestamp" is an
> > > aware
> > object (http://docs.python.org/library/datetime.html) , it's output
> > will vary depends on the input. If the timestamp is UTC time, it will
> > be return as is without convention, i.e still an aware object.
> > However, if it's not an UTC time, it will be converted to be a naive object.
> > > This mean that the function will return different type
> > > depends on
> > input, that's not so good IMHO.
> > >
> > > The worse is, any compare/substract between naïve object and
> > > aware object will fail. Because the timeutils.utcnow() and
> > > datetime.datetime.now() returns naive object, so compare/substract
> > > between the uctnow() and normalize_time() may fail, or not, depends
> > > on input from the API user. I'm a bit surprised that changes-since
> > > works on such situation :)
> > >
> > > I suspect this is caused by the "if offset". When timestamp
> > > is
> > naïve object, the offset is None. Thus check "if offset" will avoid
> > operation type exception. However, if the timestamp is UTC time, the
> > offset will be date.timeslot(0), which will return false also for "if offset".
> > >
> > > Are there any special reason that we need keep aware object
> > > if
> > input is at UTC time already? Can I changes the function to always
> > return naive object? If yes, I can create a patch for it.
> >
> > You are probably better off creating an aware datetime object, and
> > using them pervasively across the codebase, than using naive objects.
> > As you note, they are not interoperable, and I've seen countless bugs
> > where folk try to mix and match. If we want to show local date time to
> > users *anywhere*, we'll need TZ aware objects, which is why that
> > variation is the one to standardise on. Otherwise, you end up with
> > multiple conversion points, and you can guarantee that at least one will get it
> wrong.
> >
> > -Rob
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openstack
> Post to : openstack@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openstack
> More help : https://help.launchpad.net/ListHelp
Follow ups
References