Merge lp:~lifeless/python-oops-tools/bug-879309 into lp:python-oops-tools

Proposed by Robert Collins
Status: Merged
Approved by: Robert Collins
Approved revision: 6
Merged at revision: 6
Proposed branch: lp:~lifeless/python-oops-tools/bug-879309
Merge into: lp:python-oops-tools
Diff against target: 0 lines
To merge this branch: bzr merge lp:~lifeless/python-oops-tools/bug-879309
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email:

Commit message

Handle unicode urls and missing topics better.

Description of the change

This branch makes two key changes:
 - it stops forcing the pageid to be the url if there is no pageid. This will (eventually) permit dropping custom code checking for identical values in dbsummaries. It also permits events without topics to aggregate together (I think this is a good thing).

 - it treats unicode url inputs as garbage and defensively converts them to urls.

This won't help existing data, but as most reports work on new data, applying the fixes should let an existing oops-tools instance behave better from a day later.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

This looks good.

The only comment I have is an ignorable bikeshed: I had to read the
comment explaining the reason for using an empty string page ID a couple
of times before I understood it. Maybe replacing the colons with words
would help:

    # Missing topics are '' which will group all such reports together
    # in some ways, but thats tolerable vs something (near) unique like
    # a url which prevents aggregation.

review: Approve (code)
7. By Robert Collins

Reviewer suggested tweak of comment.

Preview Diff



People subscribed via source and target branches

to all changes: