Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 30037555: NoIssue - Added filterhit statistics loading capability

Created:
April 3, 2019, 4:07 p.m. by Tudor Avram
Modified:
April 3, 2019, 5:31 p.m.
Reviewers:
Vasily Kuznetsov
Visibility:
Public.

Description

NoIssue - Added filterhit statistics loading capability

Patch Set 1 #

Patch Set 2 : Removed .DS_Store files #

Total comments: 3

Patch Set 3 : Addressed Initial Comments #

Patch Set 4 : Addressed final comments (for real now) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+145 lines, -1 line) Patch
M .gitignore View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M .hgignore View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A abp/stats/__init__.py View 1 chunk +21 lines, -0 lines 0 comments Download
A abp/stats/filterhits.py View 1 2 1 chunk +52 lines, -0 lines 0 comments Download
M setup.py View 1 1 chunk +1 line, -1 line 0 comments Download
A tests/test_stats/data/filterhits.csv View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
A tests/test_stats/data/filterhits_missing_columns.csv View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
A tests/test_stats/test_filtehits_loader.py View 1 2 3 1 chunk +63 lines, -0 lines 0 comments Download

Messages

Total messages: 2
Tudor Avram
Hi Vasily, Here's the filterhits file loader. Let me know what you think Tudor.
April 3, 2019, 4:18 p.m. (2019-04-03 16:18:29 UTC) #1
Vasily Kuznetsov
April 3, 2019, 4:36 p.m. (2019-04-03 16:36:29 UTC) #2
Hey Tudor!

A couple of comments below.

Cheers,
Vasily

https://codereview.adblockplus.org/30037555/diff/30037563/.gitignore
File .gitignore (right):

https://codereview.adblockplus.org/30037555/diff/30037563/.gitignore#newcode13
.gitignore:13: .DS_Store
If we do this, we should probably just ignore .DS_Store everywhere in the tree
with one line.

https://codereview.adblockplus.org/30037555/diff/30037563/abp/stats/filterhit...
File abp/stats/filterhits.py (right):

https://codereview.adblockplus.org/30037555/diff/30037563/abp/stats/filterhit...
abp/stats/filterhits.py:38: integer_cols = ['onehour_sessions', 'hits']
What about domains and rootdomains? They seem useful too.

https://codereview.adblockplus.org/30037555/diff/30037563/tests/test_stats/da...
File tests/test_stats/data/filterhits.csv (right):

https://codereview.adblockplus.org/30037555/diff/30037563/tests/test_stats/da...
tests/test_stats/data/filterhits.csv:1: filter,source,onehour_sessions,hits
I wonder if it would make more sense to replace this file with something fake
and more compact, like 1 line from one source, 2 lines from another, super short
stuff instead real filters. What do you think?

Powered by Google App Engine
This is Rietveld