Issue 2707 - Support start/end notification parameters

sitescripts/notifications/parser.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2015 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import codecs
 import datetime
-import dateutil.parser

[Sebastian Noack, 2015/06/23 15:41:36]

Nit: Third party module imports go below corelib imports.

[Felix Dahlke, 2015/06/23 15:51:43]

Oh, didn't even realise it's third party... Done.

 import os
 import re
 import subprocess
 import tarfile
 import traceback
 from StringIO import StringIO
 
 from sitescripts.utils import get_config
 
 def _parse_targetspec(value, name):
@@ skipping 52 matching lines @@
       current["message"][locale] = value
     elif key == "target":
       target = _parse_targetspec(value, name)
       if "targets" in notification:
         current["targets"].append(target)
       else:
         current["targets"] = [target]
     elif key == "sample" and is_variant:
       current["sample"] = float(value)
     elif key in ["start", "end"]:
-      current[key] = dateutil.parser.parse(value)

[Sebastian Noack, 2015/06/23 15:41:36]

I wonder why we don't simply use a more simply format (e.g. "YYYYMMDD HH:MM")
without timezone information, always assuming UTC, like we usually do?

[Felix Dahlke, 2015/06/23 15:51:43]

Now that I know that dateutil is third party, it'd make sense to me to avoid
having to parse ISO 8601.

Wladimir, you initially suggested this - do you have a strong opinion against
using a simpler time format?

[Sebastian Noack, 2015/06/23 15:58:38]

IIRC, the initial idea was to serve one static file, having all parameters
checked on the client side. So using ISO date format would make sense, as it is
recognized by the Date object in JavaScript. However, if we process the start
and end date in Python now, a simpler format might be preferable.

[Wladimir Palant, 2015/06/23 17:24:00]

I don't think the idea was ever having this checked on the client side - client
side clocks are commonly skewed.

Frankly, the ISO format is quite common and easier to "parse" for humans than
something we invent (don't forget that these files will be edited manually).
Can't we simply parse it via datetime.strptime()?

[Felix Dahlke, 2015/06/24 07:53:16]

I really don't think we want to parse ISO 8601 ourselves here, way too much
complexity/work IMO.

But we can go with a fixed format based on an ISO 8601 subset, e.g.
"%Y-%m-%dT%H:M" ("2015-06-20T7:25"). Makes most sense to me - can still be
parsed as ISO 8601, but we don't need a third party library to support all kinds
of weird features we most likely won't need anyway. Did that.

+      current[key] = datetime.datetime.strptime(value, "%Y-%m-%dT%H:%M")
     else:
       raise Exception("Unknown parameter '%s' in file '%s'" % (key, name))
 
   for text_key in ("title", "message"):
     def has_default_locale(variant): return "en-US" in variant[text_key]
     if (not has_default_locale(notification) and
         not all(map(has_default_locale, notification.get("variants", [])))):
       raise Exception("No %s for en-US (default language) in file '%s'" %
                       (text_key, name))
   return notification
@@ skipping 20 matching lines @@
             continue
           current_time = datetime.datetime.now()
           if "start" in notification and current_time < notification["start"]:
             continue
           if "end" in notification and current_time > notification["end"]:
             continue
           notifications.append(notification)
         except:
           traceback.print_exc()
   return notifications