sitescripts/filterhits/web/submit.py - Issue 4615801646612480: Issue 395 - Filter hits statistics backend

Delta Between Two Patch Sets: sitescripts/filterhits/web/submit.py

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

Left Patch Set: Created base class for tests and reverted earlier content-type change. Created April 1, 2015, 7:01 p.m.

Right Patch Set: Addressed further comments from Sebastian. Created April 2, 2015, 10:13 a.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 # coding: utf-8	1 # coding: utf-8

2	2

3 # This file is part of the Adblock Plus web scripts,	3 # This file is part of the Adblock Plus web scripts,

4 # Copyright (C) 2006-2015 Eyeo GmbH	4 # Copyright (C) 2006-2015 Eyeo GmbH

5 #	5 #

6 # Adblock Plus is free software: you can redistribute it and/or modify	6 # Adblock Plus is free software: you can redistribute it and/or modify

7 # it under the terms of the GNU General Public License version 3 as	7 # it under the terms of the GNU General Public License version 3 as

8 # published by the Free Software Foundation.	8 # published by the Free Software Foundation.

9 #	9 #

10 # Adblock Plus is distributed in the hope that it will be useful,	10 # Adblock Plus is distributed in the hope that it will be useful,

(...skipping 75 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
86 # Update the geometrical_mean aggregations in the database	86 # Update the geometrical_mean aggregations in the database

87 interval = config.get("filterhitstats", "interval")	87 interval = config.get("filterhitstats", "interval")

88 try:	88 try:

89 db_connection = db.connect()	89 db_connection = db.connect()

90 try:	90 try:

91 db.write(db_connection, geometrical_mean.update(interval, data))	91 db.write(db_connection, geometrical_mean.update(interval, data))

92 finally:	92 finally:

93 db_connection.close()	93 db_connection.close()

94 except:	94 except:

95 # Updating the aggregations in the database failed for whatever reason,	95 # Updating the aggregations in the database failed for whatever reason,

96 # log the details but continue to return 200 OK to the client to avoid	96 # log the details but continue to return 204 to the client to avoid the

97 # re-transmission of the data.	97 # re-transmission of data.

98 processing_error_log = os.path.join(config.get("filterhitstats", "log_dir"),	98 processing_error_log = os.path.join(config.get("filterhitstats", "log_dir"),

99 "processing-errors.log")	99 "processing-errors.log")

100 with open(processing_error_log, "a+") as f:	100 with open(processing_error_log, "a+") as f:

101 message = "Problem processing data file %s:\n%s" % (	101 message = "Problem processing data file %s:\n%s" % (

102 log_file, traceback.format_exc()	102 log_file, traceback.format_exc()

103 )	103 )

104 print >> f, "[%s] %s" % (datetime.now().strftime("%d/%b/%Y:%H:%M:%S %z"), message)	104 print >> f, "[%s] %s" % (datetime.now().strftime("%d/%b/%Y:%H:%M:%S %z"), message)

105	105

106 # Send back a 200 OK response	106 # Send back a 204 No Content

107 response_headers = [("Content-type", "text/plain")]	107 start_response("204 No Content", [])

108 start_response("200 OK", response_headers)
Sebastian Noack 2015/04/02 07:37:04 Again, the status should be "204 No Content". And Again, the status should be "204 No Content". And then you shouldn't need a mime type. kzar 2015/04/02 07:47:47 What's your opinion on this Wladimir? As earlier d Show quoted text On 2015/04/02 07:37:04, Sebastian Noack wrote: > Again, the status should be "204 No Content". And then you shouldn't need a mime > type. What's your opinion on this Wladimir? As earlier discussed I would rather return 200 in case clients (wrongly) check for that explicitly. Sebastian would rather return 204 as it's a more accurate status code. Sebastian Noack 2015/04/02 08:11:44 The only client we have to bother about here is ou Show quoted text On 2015/04/02 07:47:47, kzar wrote: > On 2015/04/02 07:37:04, Sebastian Noack wrote: > > Again, the status should be "204 No Content". And then you shouldn't need a > mime > > type. > > What's your opinion on this Wladimir? As earlier discussed I would rather return > 200 in case clients (wrongly) check for that explicitly. Sebastian would rather > return 204 as it's a more accurate status code. The only client we have to bother about here is our browser extension, where we can easily make sure that it correctly handles the response status. kzar 2015/04/02 10:16:54 Fine, Done. Show quoted text On 2015/04/02 08:11:44, Sebastian Noack wrote: > On 2015/04/02 07:47:47, kzar wrote: > > On 2015/04/02 07:37:04, Sebastian Noack wrote: > > > Again, the status should be "204 No Content". And then you shouldn't need a > > mime > > > type. > > > > What's your opinion on this Wladimir? As earlier discussed I would rather > return > > 200 in case clients (wrongly) check for that explicitly. Sebastian would > rather > > return 204 as it's a more accurate status code. > > The only client we have to bother about here is our browser extension, where we > can easily make sure that it correctly handles the response status. Fine, Done.
109 return []	108 return []

LEFT	RIGHT