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

Issue 4615801646612480: Issue 395 - Filter hits statistics backend (Closed)

Created:
Dec. 19, 2014, 1:16 p.m. by kzar
Modified:
April 3, 2015, 9:48 a.m.
CC:
Felix Dahlke, saroyanm
Visibility:
Public.

Description

Issue #395 Filter hits statistics backend This code can be found in GitHub here https://github.com/kzar/adblockplus-sitescripts/tree/395-hitstats-backend . (The repository has one extra commit not included in this review that provides a Vagrant VM development environment. Clone and type `vagrant up` if you wish to test this, you can then access the webserver at http://localhost:5000 and the query interface at http://localhost:5000/static/query.html ) In case it helps I have also written some helpful utilities for generating dummy data and dumping the logged data into a flattened JSON format. These are not part of the sitescripts repository or this review but can be obtained separately here https://github.com/kzar/adblockplus-filterhits-helpers The process_logs.py script included is a convenient way to re-load log files into the database. I included this as we are expecting the aggregation method to be adjusted in the future and this provides an easy way to then re-load the data. It can load either a single log file or all the log files in a directory, all as one transaction which is rolled back in case of error.

Patch Set 1 #

Total comments: 71

Patch Set 2 : Improvements regarding comments #

Total comments: 35

Patch Set 3 : Addressed comments. #

Patch Set 4 : Added API tests, addressed comments and some other improvements. #

Total comments: 14

Patch Set 5 : Addressed more comments. #

Patch Set 6 : Display friendly message if processing script can't connect to DB. #

Patch Set 7 : Rebased. #

Patch Set 8 : Simplified geometrical_mean code and reduced filter inserts. #

Total comments: 44

Patch Set 9 : Addressed some of Wladimir's comments #

Total comments: 2

Patch Set 10 : Addressed Sebastian's and Wladimir's comments. #

Total comments: 32

Patch Set 11 : Addressed further comments from Wladimir. #

Total comments: 8

Patch Set 12 : Addressed final nits. #

Patch Set 13 : Rebased. #

Patch Set 14 : Removed db.testing flag, instead overwrite config during testing. #

Total comments: 4

Patch Set 15 : Make test log directory path configurable and ensure it's always cleared. #

Total comments: 2

Patch Set 16 : Create temporary log directory with tempfile module for tests. #

Total comments: 2

Patch Set 17 : Make sure the temporary log directory is recreated for each test. #

Total comments: 20

Patch Set 18 : Addressed further feedback from Sebastian. #

Patch Set 19 : Created base class for tests and reverted earlier content-type change. #

Total comments: 12

Patch Set 20 : Addressed further comments from Sebastian. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1157 lines, -9 lines) Patch
M .sitescripts.example View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +12 lines, -0 lines 0 comments Download
A + sitescripts/filterhits/__init__.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 0 chunks +-1 lines, --1 lines 0 comments Download
A + sitescripts/filterhits/bin/__init__.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 0 chunks +-1 lines, --1 lines 0 comments Download
A sitescripts/filterhits/bin/reprocess_logs.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +83 lines, -0 lines 0 comments Download
A sitescripts/filterhits/db.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +67 lines, -0 lines 0 comments Download
A sitescripts/filterhits/geometrical_mean.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +45 lines, -0 lines 0 comments Download
A sitescripts/filterhits/schema.sql View 1 2 3 4 5 6 7 8 9 10 1 chunk +16 lines, -0 lines 0 comments Download
A sitescripts/filterhits/static/datatables/jquery.js View 2 1 chunk +4 lines, -0 lines 0 comments Download
A sitescripts/filterhits/static/datatables/jquery.dataTables.min.css View 2 1 chunk +1 line, -0 lines 0 comments Download
A sitescripts/filterhits/static/datatables/jquery.dataTables.min.js View 2 1 chunk +157 lines, -0 lines 0 comments Download
A sitescripts/filterhits/static/datatables/sort_asc.png View 2 Binary file 0 comments Download
A sitescripts/filterhits/static/datatables/sort_both.png View 2 Binary file 0 comments Download
A sitescripts/filterhits/static/datatables/sort_desc.png View 2 Binary file 0 comments Download
A sitescripts/filterhits/static/query.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +31 lines, -0 lines 0 comments Download
A sitescripts/filterhits/static/query.js View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +39 lines, -0 lines 0 comments Download
A + sitescripts/filterhits/test/__init__.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 0 chunks +-1 lines, --1 lines 0 comments Download
A sitescripts/filterhits/test/api_tests.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +171 lines, -0 lines 0 comments Download
A sitescripts/filterhits/test/db_tests.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +49 lines, -0 lines 0 comments Download
A sitescripts/filterhits/test/geometrical_mean_tests.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +166 lines, -0 lines 0 comments Download
A sitescripts/filterhits/test/log_tests.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +55 lines, -0 lines 0 comments Download
A sitescripts/filterhits/test/test_helpers.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +70 lines, -0 lines 0 comments Download
A + sitescripts/filterhits/web/__init__.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 0 chunks +-1 lines, --1 lines 0 comments Download
A + sitescripts/filterhits/web/common.py View 1 2 3 4 5 6 7 8 9 10 1 chunk +3 lines, -13 lines 0 comments Download
A sitescripts/filterhits/web/query.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +84 lines, -0 lines 0 comments Download
A sitescripts/filterhits/web/submit.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +108 lines, -0 lines 0 comments Download

Messages

Total messages: 59
kzar
Dec. 19, 2014, 1:27 p.m. (2014-12-19 13:27:40 UTC) #1
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/.hgignore File .hgignore (right): http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/.hgignore#newcode6 .hgignore:6: sitescripts/filterhits/test/temp While on it mind adding a .gitignore like ...
Feb. 11, 2015, 4 p.m. (2015-02-11 16:00:11 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/bin/process_logs.py#newcode33 sitescripts/filterhits/bin/process_logs.py:33: if f.endswith(".log") and f[0].isdigit(): Nit: Please use os.path.splitext() http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/bin/process_logs.py#newcode46 ...
Feb. 11, 2015, 4:29 p.m. (2015-02-11 16:29:32 UTC) #3
kzar
http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/.hgignore File .hgignore (right): http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/.hgignore#newcode6 .hgignore:6: sitescripts/filterhits/test/temp On 2015/02/11 16:00:12, Sebastian Noack wrote: > While ...
Feb. 17, 2015, 10:52 a.m. (2015-02-17 10:52:23 UTC) #4
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/db.py File sitescripts/filterhits/db.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/db.py#newcode68 sitescripts/filterhits/db.py:68: if isinstance(sql, str): On 2015/02/11 16:00:12, Sebastian Noack wrote: ...
Feb. 17, 2015, 2:59 p.m. (2015-02-17 14:59:17 UTC) #5
Wladimir Palant
http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/test/common_tests.py File sitescripts/filterhits/test/common_tests.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/test/common_tests.py#newcode1 sitescripts/filterhits/test/common_tests.py:1: # coding: utf-8 On 2015/02/11 16:00:12, Sebastian Noack wrote: ...
Feb. 17, 2015, 3:12 p.m. (2015-02-17 15:12:20 UTC) #6
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5673385510043648/sitescripts/filterhits/common.py File sitescripts/filterhits/common.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5673385510043648/sitescripts/filterhits/common.py#newcode30 sitescripts/filterhits/common.py:30: now = time.gmtime() On 2015/02/17 15:12:21, Wladimir Palant wrote: ...
Feb. 17, 2015, 3:19 p.m. (2015-02-17 15:19:43 UTC) #7
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5673385510043648/sitescripts/filterhits/web/submit.py File sitescripts/filterhits/web/submit.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5673385510043648/sitescripts/filterhits/web/submit.py#newcode45 sitescripts/filterhits/web/submit.py:45: data = json.loads(data) data = {} try: data_length = ...
Feb. 17, 2015, 5:09 p.m. (2015-02-17 17:09:47 UTC) #8
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5673385510043648/sitescripts/filterhits/web/submit.py File sitescripts/filterhits/web/submit.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5673385510043648/sitescripts/filterhits/web/submit.py#newcode45 sitescripts/filterhits/web/submit.py:45: data = json.loads(data) Possibly you could also just use ...
Feb. 17, 2015, 5:16 p.m. (2015-02-17 17:16:57 UTC) #9
kzar
http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/db.py File sitescripts/filterhits/db.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/db.py#newcode68 sitescripts/filterhits/db.py:68: if isinstance(sql, str): On 2015/02/17 14:59:17, Sebastian Noack wrote: ...
Feb. 24, 2015, 6:05 p.m. (2015-02-24 18:05:11 UTC) #10
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/db.py File sitescripts/filterhits/db.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/db.py#newcode68 sitescripts/filterhits/db.py:68: if isinstance(sql, str): On 2015/02/24 18:05:11, kzar wrote: > ...
Feb. 26, 2015, 4:39 p.m. (2015-02-26 16:39:24 UTC) #11
kzar
Added API tests, addressed comments and some other improvements. http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/db.py File sitescripts/filterhits/db.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5629499534213120/sitescripts/filterhits/db.py#newcode68 sitescripts/filterhits/db.py:68: ...
Feb. 28, 2015, 7:39 p.m. (2015-02-28 19:39:56 UTC) #12
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5673385510043648/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5673385510043648/sitescripts/filterhits/bin/process_logs.py#newcode46 sitescripts/filterhits/bin/process_logs.py:46: while not (last == '"' and current == " ...
March 2, 2015, 10:04 a.m. (2015-03-02 10:04:01 UTC) #13
kzar
Patch Set 5 : Addressed more comments. http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py#newcode20 sitescripts/filterhits/bin/process_logs.py:20: import sitescripts.filterhits.common ...
March 2, 2015, 10:39 a.m. (2015-03-02 10:39:03 UTC) #14
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py#newcode89 sitescripts/filterhits/bin/process_logs.py:89: if db_connection: On 2015/03/02 10:39:03, kzar wrote: > On ...
March 2, 2015, 10:56 a.m. (2015-03-02 10:56:35 UTC) #15
kzar
On 2015/03/02 10:56:35, Sebastian Noack wrote: > http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py > File sitescripts/filterhits/bin/process_logs.py (right): > > http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py#newcode89 ...
March 2, 2015, 10:58 a.m. (2015-03-02 10:58:40 UTC) #16
kzar
Sorry, I hit the wrong reply button there. Ignore the last email. http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py ...
March 2, 2015, 11 a.m. (2015-03-02 11:00:52 UTC) #17
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py#newcode89 sitescripts/filterhits/bin/process_logs.py:89: if db_connection: On 2015/03/02 11:00:52, kzar wrote: > On ...
March 2, 2015, 11:06 a.m. (2015-03-02 11:06:05 UTC) #18
kzar
Patch Set 6 : Display friendly message if processing script can't connect to DB. http://codereview.adblockplus.org/4615801646612480/diff/5768755258851328/sitescripts/filterhits/bin/process_logs.py ...
March 2, 2015, 11:18 a.m. (2015-03-02 11:18:50 UTC) #19
Sebastian Noack
LGTM. But I suppose Wladimir wants to have a closer look himself before I merge ...
March 2, 2015, 11:27 a.m. (2015-03-02 11:27:52 UTC) #20
kzar
Patch Set 7 : Rebased.
March 2, 2015, 8:42 p.m. (2015-03-02 20:42:28 UTC) #21
kzar
Patch Set 8 : Simplified geometrical_mean code and reduced filter inserts.
March 16, 2015, 4:25 p.m. (2015-03-16 16:25:41 UTC) #22
Wladimir Palant
This is only a partial review, I didn't get to reviewing the web handlers yet. ...
March 26, 2015, 10:56 p.m. (2015-03-26 22:56:50 UTC) #23
kzar
Patch Set 9 : Addressed some of Wladimir's comments http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/.sitescripts.example File .sitescripts.example (right): http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/.sitescripts.example#newcode178 .sitescripts.example:178: ...
March 27, 2015, 11:59 a.m. (2015-03-27 11:59:57 UTC) #24
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/bin/process_logs.py#newcode18 sitescripts/filterhits/bin/process_logs.py:18: import MySQLdb, itertools, json, os, sys On 2015/03/27 11:59:57, ...
March 27, 2015, 1:12 p.m. (2015-03-27 13:12:19 UTC) #25
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/db.py File sitescripts/filterhits/db.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/db.py#newcode28 sitescripts/filterhits/db.py:28: db=config.get("filterhitstats", "test_database" if testing else "database"), On 2015/03/27 13:12:19, ...
March 27, 2015, 2:26 p.m. (2015-03-27 14:26:06 UTC) #26
kzar
Patch Set 10 : Addressed Sebastian's and Wladimir's comments. http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/bin/process_logs.py#newcode18 sitescripts/filterhits/bin/process_logs.py:18: ...
March 27, 2015, 3:10 p.m. (2015-03-27 15:10:50 UTC) #27
Wladimir Palant
Done with the review. Note that I only skimmed the tests, might take a closer ...
March 27, 2015, 4:29 p.m. (2015-03-27 16:29:06 UTC) #28
Wladimir Palant
http://codereview.adblockplus.org/4615801646612480/diff/5755424754106368/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5755424754106368/sitescripts/filterhits/bin/process_logs.py#newcode1 sitescripts/filterhits/bin/process_logs.py:1: # coding: utf-8 One more note: the purpose of ...
March 27, 2015, 4:30 p.m. (2015-03-27 16:30:48 UTC) #29
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5755424754106368/sitescripts/filterhits/web/submit.py File sitescripts/filterhits/web/submit.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5755424754106368/sitescripts/filterhits/web/submit.py#newcode53 sitescripts/filterhits/web/submit.py:53: print >> f, json.dumps(data) json.dump(data, f)
March 27, 2015, 4:31 p.m. (2015-03-27 16:31:04 UTC) #30
kzar
Patch Set 11 : Addressed further comments from Wladimir. http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/bin/process_logs.py File sitescripts/filterhits/bin/process_logs.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/bin/process_logs.py#newcode31 sitescripts/filterhits/bin/process_logs.py:31: ...
March 27, 2015, 10:15 p.m. (2015-03-27 22:15:00 UTC) #31
Wladimir Palant
Almost there, only a few nit left. http://codereview.adblockplus.org/4615801646612480/diff/5735425238892544/.hgignore File .hgignore (right): http://codereview.adblockplus.org/4615801646612480/diff/5735425238892544/.hgignore#newcode5 .hgignore:5: dist Now ...
March 28, 2015, 12:59 p.m. (2015-03-28 12:59:19 UTC) #32
kzar
Patch Set 12 : Addressed final nits. Patch Set 13 : Rebased. http://codereview.adblockplus.org/4615801646612480/diff/5735425238892544/.hgignore File .hgignore ...
March 28, 2015, 2:11 p.m. (2015-03-28 14:11:56 UTC) #33
Wladimir Palant
LGTM http://codereview.adblockplus.org/4615801646612480/diff/5735425238892544/sitescripts/filterhits/web/common.py File sitescripts/filterhits/web/common.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5735425238892544/sitescripts/filterhits/web/common.py#newcode1 sitescripts/filterhits/web/common.py:1: # coding: utf-8 On 2015/03/28 14:11:56, kzar wrote: ...
March 29, 2015, 7:29 a.m. (2015-03-29 07:29:11 UTC) #34
kzar
http://codereview.adblockplus.org/4615801646612480/diff/5735425238892544/sitescripts/filterhits/web/common.py File sitescripts/filterhits/web/common.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5735425238892544/sitescripts/filterhits/web/common.py#newcode1 sitescripts/filterhits/web/common.py:1: # coding: utf-8 On 2015/03/29 07:29:12, Wladimir Palant wrote: ...
March 29, 2015, 11:23 a.m. (2015-03-29 11:23:40 UTC) #35
Wladimir Palant
Sorry, not ready after all. The way unit tests work still needs to be fixed. ...
March 30, 2015, 11:23 a.m. (2015-03-30 11:23:55 UTC) #36
kzar
Patch Set 14 : Removed db.testing flag, instead overwrite config during testing. http://codereview.adblockplus.org/4615801646612480/diff/5659118702428160/sitescripts/filterhits/db.py File sitescripts/filterhits/db.py ...
March 30, 2015, 1:01 p.m. (2015-03-30 13:01:42 UTC) #37
Wladimir Palant
http://codereview.adblockplus.org/4615801646612480/diff/5154545407623168/sitescripts/filterhits/test/test_helpers.py File sitescripts/filterhits/test/test_helpers.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5154545407623168/sitescripts/filterhits/test/test_helpers.py#newcode34 sitescripts/filterhits/test/test_helpers.py:34: config.set("filterhitstats", "log_dir", test_config["log_dir"]) Maybe create a real temporary directory ...
March 30, 2015, 1:13 p.m. (2015-03-30 13:13:46 UTC) #38
kzar
Patch Set 15 : Make test log directory path configurable and ensure it's always cleared. ...
March 30, 2015, 3:14 p.m. (2015-03-30 15:14:53 UTC) #39
Wladimir Palant
http://codereview.adblockplus.org/4615801646612480/diff/4815735301865472/sitescripts/filterhits/test/test_helpers.py File sitescripts/filterhits/test/test_helpers.py (right): http://codereview.adblockplus.org/4615801646612480/diff/4815735301865472/sitescripts/filterhits/test/test_helpers.py#newcode29 sitescripts/filterhits/test/test_helpers.py:29: "log_dir": config.get("filterhitstats", "test_log_dir") Why configure that directory? setup_config() can ...
March 30, 2015, 6:35 p.m. (2015-03-30 18:35:53 UTC) #40
kzar
Patch Set 16 : Create temporary log directory with tempfile module for tests. http://codereview.adblockplus.org/4615801646612480/diff/4815735301865472/sitescripts/filterhits/test/test_helpers.py File ...
March 30, 2015, 7:01 p.m. (2015-03-30 19:01:34 UTC) #41
Wladimir Palant
http://codereview.adblockplus.org/4615801646612480/diff/5633494390669312/sitescripts/filterhits/test/test_helpers.py File sitescripts/filterhits/test/test_helpers.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5633494390669312/sitescripts/filterhits/test/test_helpers.py#newcode30 sitescripts/filterhits/test/test_helpers.py:30: "log_dir": tempfile.mkdtemp() This will create the directory exactly once ...
March 30, 2015, 7:07 p.m. (2015-03-30 19:07:03 UTC) #42
kzar
Patch Set 17 : Make sure the temporary log directory is recreated for each test. ...
March 30, 2015, 7:29 p.m. (2015-03-30 19:29:02 UTC) #43
Wladimir Palant
LGTM
March 30, 2015, 8:22 p.m. (2015-03-30 20:22:48 UTC) #44
Sebastian Noack
Looks mostly good. Only a few nits left. http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/db.py File sitescripts/filterhits/db.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/db.py#newcode38 sitescripts/filterhits/db.py:38: if ...
March 31, 2015, 7:55 a.m. (2015-03-31 07:55:21 UTC) #45
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/test/log_tests.py File sitescripts/filterhits/test/log_tests.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/test/log_tests.py#newcode64 sitescripts/filterhits/test/log_tests.py:64: if __name__ == '__main__': On 2015/03/31 07:55:21, Sebastian Noack ...
March 31, 2015, 9:20 a.m. (2015-03-31 09:20:18 UTC) #46
kzar
Patch Set 18 : Addressed further feedback from Sebastian. http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/db.py File sitescripts/filterhits/db.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/db.py#newcode38 sitescripts/filterhits/db.py:38: ...
March 31, 2015, 9:48 a.m. (2015-03-31 09:48:56 UTC) #47
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/test/log_tests.py File sitescripts/filterhits/test/log_tests.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/test/log_tests.py#newcode31 sitescripts/filterhits/test/log_tests.py:31: self.config = test_helpers.setup_config() On 2015/03/31 09:48:56, kzar wrote: > ...
March 31, 2015, 10:19 a.m. (2015-03-31 10:19:03 UTC) #48
kzar
http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/test/log_tests.py File sitescripts/filterhits/test/log_tests.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/test/log_tests.py#newcode31 sitescripts/filterhits/test/log_tests.py:31: self.config = test_helpers.setup_config() On 2015/03/31 10:19:04, Sebastian Noack wrote: ...
March 31, 2015, 10:27 a.m. (2015-03-31 10:27:46 UTC) #49
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/test/log_tests.py File sitescripts/filterhits/test/log_tests.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/test/log_tests.py#newcode31 sitescripts/filterhits/test/log_tests.py:31: self.config = test_helpers.setup_config() On 2015/03/31 10:27:47, kzar wrote: > ...
March 31, 2015, 10:32 a.m. (2015-03-31 10:32:22 UTC) #50
Wladimir Palant
http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/web/submit.py File sitescripts/filterhits/web/submit.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/web/submit.py#newcode106 sitescripts/filterhits/web/submit.py:106: response_headers = [("Content-type", "text/plain")] On 2015/03/31 07:55:21, Sebastian Noack ...
March 31, 2015, 1:42 p.m. (2015-03-31 13:42:31 UTC) #51
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/web/submit.py File sitescripts/filterhits/web/submit.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5722383033827328/sitescripts/filterhits/web/submit.py#newcode106 sitescripts/filterhits/web/submit.py:106: response_headers = [("Content-type", "text/plain")] On 2015/03/31 13:42:31, Wladimir Palant ...
March 31, 2015, 1:46 p.m. (2015-03-31 13:46:53 UTC) #52
kzar
Patch Set 19 : Created base class for tests and reverted earlier content-type change. (Sorry ...
April 1, 2015, 7:09 p.m. (2015-04-01 19:09:39 UTC) #53
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5713050069893120/sitescripts/filterhits/test/test_helpers.py File sitescripts/filterhits/test/test_helpers.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5713050069893120/sitescripts/filterhits/test/test_helpers.py#newcode45 sitescripts/filterhits/test/test_helpers.py:45: # Set up test config There is a lot ...
April 2, 2015, 7:37 a.m. (2015-04-02 07:37:04 UTC) #54
kzar
http://codereview.adblockplus.org/4615801646612480/diff/5713050069893120/sitescripts/filterhits/test/test_helpers.py File sitescripts/filterhits/test/test_helpers.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5713050069893120/sitescripts/filterhits/test/test_helpers.py#newcode45 sitescripts/filterhits/test/test_helpers.py:45: # Set up test config On 2015/04/02 07:37:04, Sebastian ...
April 2, 2015, 7:47 a.m. (2015-04-02 07:47:46 UTC) #55
Sebastian Noack
http://codereview.adblockplus.org/4615801646612480/diff/5713050069893120/sitescripts/filterhits/test/test_helpers.py File sitescripts/filterhits/test/test_helpers.py (right): http://codereview.adblockplus.org/4615801646612480/diff/5713050069893120/sitescripts/filterhits/test/test_helpers.py#newcode45 sitescripts/filterhits/test/test_helpers.py:45: # Set up test config On 2015/04/02 07:47:47, kzar ...
April 2, 2015, 8:11 a.m. (2015-04-02 08:11:44 UTC) #56
kzar
Patch Set 20 : Addressed further comments from Sebastian. (Ignore "change" to __init__.py files, the ...
April 2, 2015, 10:16 a.m. (2015-04-02 10:16:54 UTC) #57
Sebastian Noack
LGTM
April 2, 2015, 11 a.m. (2015-04-02 11:00:41 UTC) #58
Wladimir Palant
April 2, 2015, 5:26 p.m. (2015-04-02 17:26:28 UTC) #59
LGTM

Powered by Google App Engine
This is Rietveld