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 | ||||
| Related bugs: |
|
| 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.
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.