Merge lp:~barry/gwibber/bug-990145 into lp:gwibber
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Merged at revision: | 1354 | ||||
| Proposed branch: | lp:~barry/gwibber/bug-990145 | ||||
| Merge into: | lp:gwibber | ||||
| Diff against target: |
697 lines (+286/-63) 16 files modified
README (+50/-0) gwibber/microblog/dispatcher.py (+2/-2) gwibber/microblog/plugins/facebook/__init__.py (+27/-8) gwibber/microblog/plugins/flickr/__init__.py (+3/-6) gwibber/microblog/plugins/friendfeed/__init__.py (+3/-2) gwibber/microblog/plugins/identica/__init__.py (+7/-5) gwibber/microblog/plugins/qaiku/__init__.py (+2/-1) gwibber/microblog/plugins/statusnet/__init__.py (+7/-6) gwibber/microblog/plugins/twitter/__init__.py (+7/-5) gwibber/microblog/util/__init__.py (+3/-9) gwibber/microblog/util/resources.py (+2/-2) gwibber/time.py (+96/-0) gwibber/util.py (+3/-3) tests/plugins/test/__init__.py (+3/-5) tests/python/unittests/test_time.py (+53/-0) tests/python/utils/__init__.py (+18/-9) |
||||
| To merge this branch: | bzr merge lp:~barry/gwibber/bug-990145 | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Ken VanDine | 2012-04-27 | Approve on 2012-05-22 | |
|
Review via email:
|
|||
Description of the Change
It's a release goal for Quantal to ship only Python 3 on the desktop CD, so we
need to port Gwibber to Python 3. This branch doesn't do that. Instead, it
removes a dependency which we know will not be ported upstream any time soon.
I've heard from the mx project that they have no plans to port to Python 3.
This branch changes the uses of mx.DateTime to the built-in datetime module.
A 'make check' seems to pass locally, but it's entirely possible that I'm not
running the full suite correctly. I'm happy to make any additional changes
that might be necessary.
| Barry Warsaw (barry) wrote : | # |
| Ken VanDine (ken-vandine) wrote : | # |
Thanks for the branch, review looks good and the unit tests pass. However it does fail with twitter, unfortunately we don't have tests that use real data from the services. It looks like it needs special handling like you did in facebook. Here is the traceback:
Dispatcher Thread-1 : ERROR <twitter:receive> Operation failed
Dispatcher Thread-1 : DEBUG Traceback:
Traceback (most recent call last):
File "/home/
message_data = PROTOCOLS[
File "/home/
return getattr(self, opname)(**args)
File "/home/
return self._get(
File "/home/
if parse: return [getattr(self, "_%s" % parse)(m) for m in data]
File "/home/
n["time"] = util.parsetime(
File "/home/
dt = datetime.
File "/usr/lib/
(data_string, format))
ValueError: time data 'Thu May 10 17:52:38 +0000 2012' does not match format '%Y-%m-%dT%H:%M:%S'
| Ken VanDine (ken-vandine) wrote : | # |
Actually my last test didn't download any updates for facebook, so I purged all my facebook messages from the database. Now facebook refresh gives me this traceback:
Dispatcher Thread-6 : ERROR <facebook:receive> Operation failed
Dispatcher Thread-6 : DEBUG Traceback:
Traceback (most recent call last):
File "/home/
message_data = PROTOCOLS[
File "/home/
return getattr(self, opname)(**args)
File "/home/
return [self._
File "/home/
m["time"] = convert_time(data)
File "/home/
as_datetime = datetime.
File "/usr/lib/
data_
ValueError: unconverted data remains: +0000
- 1345. By Barry Warsaw on 2012-05-11
-
- Add the most minimal of unittests, with instructions for running them in
the README file.- Add unittests for parsetime().
- Support more formats in parsetime(), e.g. timezone aware strings (but only
for UTC, i.e. +0000), and also for alternative ISO 8601 (space instead of
'T').
| Barry Warsaw (barry) wrote : | # |
I've pushed an update which should fix the parsing in the last comment. I also added the most minimal of unittests, based on our out-of-band discussion. See the README for details. It's ugly and not tied into `make check` but at least it's a start. ;)
- 1346. By Barry Warsaw on 2012-05-11
-
trunk merge
| Barry Warsaw (barry) wrote : | # |
I think the last update may not completely fix this problem. I'm working on an update.
- 1347. By Barry Warsaw on 2012-05-11
-
- Remove some unused imports, and other import cleanups.
- Avoid circular imports by moving parsetime() to new module gwibber.time
- Refactor convert_time() to use parsetime() in order to fix the bugs
everywhere.
| Barry Warsaw (barry) wrote : | # |
Latest update pushed, which should work (seems to locally anyway ;). You'll see the diff got bigger because I had to fiddle with the imports to prevent some circular import problems.
- 1348. By Barry Warsaw on 2012-05-11
-
- Handle Twitter and Facebook nonconformity to standards.
- Refactor the code to be more readable.
- Improve the comments.
| Ken VanDine (ken-vandine) wrote : | # |
twitter and facebook confirmed to work, however I tested with identica and status.net and they aren't using a UTC time and give the same ValueError. Here is an example time string:
Wed Mar 21 02:30:31 -0400 2012
There is also a typo in the flickr plugin:
- timetup = datetime.
+ timetup = datetime.
The good news is that will be the last of the supported plugins, so once identica and statusnet are working this is good to go!
Thanks!
| Barry Warsaw (barry) wrote : | # |
On May 11, 2012, at 11:42 PM, Ken VanDine wrote:
>Review: Needs Fixing
>
>twitter and facebook confirmed to work, however I tested with identica and
>status.net and they aren't using a UTC time and give the same ValueError.
>Here is an example time string:
>
>Wed Mar 21 02:30:31 -0400 2012
Madness!
What does Gwibber expect us to do with non-UTC timezones? Just dropping the
tz doesn't seem right. OTOH, we have to convert them to naive datetimes. So
I'm guessing we parse them into non-UTC tz-aware datetimes, then convert them
to UTC, then make them tz-naive. Does that sound right?
>There is also a typo in the flickr plugin:
>- timetup = datetime.
>+ timetup = datetime.
>
>The good news is that will be the last of the supported plugins, so once
>identica and statusnet are working this is good to go!
Yay! :)
Cheers,
-Barry
| Ken VanDine (ken-vandine) wrote : | # |
On Fri, 2012-05-11 at 16:52 -0700, Barry Warsaw wrote:
> On May 11, 2012, at 11:42 PM, Ken VanDine wrote:
>
> >Review: Needs Fixing
> >
> >twitter and facebook confirmed to work, however I tested with identica and
> >status.net and they aren't using a UTC time and give the same ValueError.
> >Here is an example time string:
> >
> >Wed Mar 21 02:30:31 -0400 2012
>
> Madness!
>
> What does Gwibber expect us to do with non-UTC timezones? Just dropping the
> tz doesn't seem right. OTOH, we have to convert them to naive datetimes. So
> I'm guessing we parse them into non-UTC tz-aware datetimes, then convert them
> to UTC, then make them tz-naive. Does that sound right?
Yeah, I think that is the only thing we can do. Sure wish they were
consistent :(
Thanks!
>
> >There is also a typo in the flickr plugin:
> >- timetup = datetime.
> >+ timetup = datetime.
> >
> >The good news is that will be the last of the supported plugins, so once
> >identica and statusnet are working this is good to go!
>
> Yay! :)
>
> Cheers,
> -Barry
>
--
Ken VanDine
Ubuntu Desktop Integration Engineer
Canonical, Ltd.
- 1349. By Barry Warsaw on 2012-05-12
-
- Change the flickr plugin to use the parsetime() utility.
- Remove some unused imports from the flickr plugin.
- parsetime(): Elaborate so that non-UTC timezones are handled properly,
which is used by identica and status.net. We have to use regexps here
though since %z isn't universally supported until Python 3.
| Barry Warsaw (barry) wrote : | # |
On May 11, 2012, at 11:42 PM, Ken VanDine wrote:
>twitter and facebook confirmed to work, however I tested with identica and
>status.net and they aren't using a UTC time and give the same ValueError.
>Here is an example time string:
>
>Wed Mar 21 02:30:31 -0400 2012
Okay, I think I'm handling this case now. Manual timezone math is always fun
(you have to subtract the value from your local time to get you to UTC).
Please do check my math! (And the test cases. :)
>There is also a typo in the flickr plugin:
>- timetup = datetime.
>+ timetup = datetime.
I changed this to use gwibber.
>The good news is that will be the last of the supported plugins, so once
>identica and statusnet are working this is good to go!
Great! Hopefully this time <wink> will do it.
Update pushed.


On May 10, 2012, at 06:09 PM, Ken VanDine wrote:
>Review: Needs Fixing
>
>Actually my last test didn't download any updates for facebook, so I purged
>all my facebook messages from the database. Now facebook refresh gives me
>this traceback:
Ah, so the FB data does have a timezone. Seems like a shallow bug, should be
easy to fix. I'll do that asap.
-Barry
> ken/src/ gwibber/ trunk/gwibber/ microblog/ dispatcher. py", line 81, in run account[ "service" ]].Client( account) (opname, **args) ken/src/ gwibber/ trunk/gwibber/ microblog/ plugins/ facebook/ __init_ _.py", line 276, in __call__ ken/src/ gwibber/ trunk/gwibber/ microblog/ plugins/ facebook/ __init_ _.py", line 292, in receive message( post) for post in data["data"]] ken/src/ gwibber/ trunk/gwibber/ microblog/ plugins/ facebook/ __init_ _.py", line 151, in _message ken/src/ gwibber/ trunk/gwibber/ microblog/ plugins/ facebook/ __init_ _.py", line 80, in convert_time strptime( iso8601, ISO8601FORMAT) python2. 7/_strptime. py", line 328, in _strptime found.end( ):])
>
>Dispatcher Thread-6 : ERROR <facebook:receive> Operation failed
>Dispatcher Thread-6 : DEBUG Traceback:
>Traceback (most recent call last):
> File "/home/
> message_data = PROTOCOLS[
> File "/home/
> return getattr(self, opname)(**args)
> File "/home/
> return [self._
> File "/home/
> m["time"] = convert_time(data)
> File "/home/
> as_datetime = datetime.
> File "/usr/lib/
> data_string[
>ValueError: unconverted data remains: +0000
>