Eoghan, thanks for you comments. I will change it to openstack-common. --jyh
> -----Original Message----- > From: Eoghan Glynn [mailto:[email protected]] > Sent: Monday, September 10, 2012 10:33 PM > To: Jiang, Yunhong > Cc: [email protected]; Robert Collins > Subject: Re: [Openstack] One potential issue on the normalize_time() in > timeutils > > > Thanks Yunhong for pointing this issue out and submitting a patch in quick > order. > > Your reasoning for switching from if offset to if offset is None, in order to > avoid > including the offset==0 case, makes perfect sense. > > You'll just have to propose the change first to openstack-common, from where > it will be copied to the nova, glance etc. codebases. > > Cheers, > Eoghan > > > ----- Original Message ----- > > I create a patch for it https://review.openstack.org/#/c/12705/ and > > please help to review. > > > > Thanks > > --jyh > > > > > -----Original Message----- > > > From: [email protected] > > > [mailto:[email protected] > > > t] > > > On > > > Behalf Of Jiang, Yunhong > > > Sent: Monday, September 10, 2012 8:17 AM > > > To: Robert Collins > > > Cc: [email protected] > > > 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:[email protected]] > > > > Sent: Monday, September 10, 2012 3:25 AM > > > > To: Jiang, Yunhong > > > > Cc: [email protected]; [email protected] > > > > Subject: Re: [Openstack] One potential issue on the > > > > normalize_time() > > > > in timeutils > > > > > > > > On Mon, Sep 10, 2012 at 3:09 AM, Jiang, Yunhong > > > > <[email protected]> > > > > 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 : [email protected] > > > Unsubscribe : https://launchpad.net/~openstack > > > More help : https://help.launchpad.net/ListHelp > > > > _______________________________________________ > > Mailing list: https://launchpad.net/~openstack > > Post to : [email protected] > > Unsubscribe : https://launchpad.net/~openstack > > More help : https://help.launchpad.net/ListHelp > > _______________________________________________ Mailing list: https://launchpad.net/~openstack Post to : [email protected] Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
