Issue 3774 - Support multiple mirrors for the Malware Domains List

sitescripts/subscriptions/bin/updateMalwareDomainsList.py

Index: sitescripts/subscriptions/bin/updateMalwareDomainsList.py
===================================================================
--- a/sitescripts/subscriptions/bin/updateMalwareDomainsList.py
+++ b/sitescripts/subscriptions/bin/updateMalwareDomainsList.py
@@ -16,68 +16,61 @@
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import os, subprocess, codecs, urllib2, zipfile, tempfile, shutil, sys
 import ConfigParser
 from StringIO import StringIO
 from sitescripts.utils import get_config
 
 
-filterlist_header = '''[Adblock Plus 1.1]
+FILTERLIST_HEADER = '''[Adblock Plus 1.1]
 ! This is a list of malware domains generated from malwaredomains.com data.
 ! Homepage: http://malwaredomains.com/?page_id=2
 ! Last modified: %timestamp%
 ! Expires: 1d
 !'''
 
-# Default value for malwaredomains_mirrors in the config.
-# Equivalent to the following in the config:
-# malwaredomains_mirrors=
-#     http://mirror3.malwaredomains.com
-#     http://mirror1.malwaredomains.com
-#     http://mirror2.malwaredomains.com
-default_mirrors_list = [
+DEFAULT_MIRRORS_LIST = [
     'http://mirror3.malwaredomains.com',
     'http://mirror1.malwaredomains.com',
     'http://mirror2.malwaredomains.com'
 ]
 
-malwaredomains_path = '/files/justdomains.zip'
+MALWAREDOMAINS_PATH = '/files/justdomains.zip'
 
 
 def try_mirror(mirror, path):
   try:
     response = urllib2.urlopen(mirror + path)
     return response.read()
-  except urllib2.HTTPError, err:
-    print >>sys.stderr, '{}: {}'.format(mirror, err)
+  except urllib2.HTTPError:
     return None
 
 
 if __name__ == '__main__':
   config = get_config()
   repository = config.get('subscriptionDownloads', 'malwaredomains_repository')
   try:
     mirrors = config.get('subscriptionDownloads', 'malwaredomains_mirrors')
     mirrors_list = filter(None, [mirror.strip() for mirror in mirrors.split()])

[Sebastian Noack, 2016/03/14 15:40:12]

strip() and filter() is redundant. If you use split() without arguments this is
equivalent to re.split(r"\s+", s.strip())

[Vasily Kuznetsov, 2016/03/14 16:20:38]

Yeah, strip is definitely redundant. And I was thinking that something like '  
foo   bar   '.split() would produce ['', 'foo', 'bar', ''] but you're right that
it doesn't. Fixed.

   except ConfigParser.NoOptionError:
-    mirrors_list = default_mirrors_list
+    mirrors_list = DEFAULT_MIRRORS_LIST
 
   tempdir = tempfile.mkdtemp(prefix='malwaredomains')
   try:
     subprocess.check_call(['hg', '-q', 'clone',  '-U', repository, tempdir])
     subprocess.check_call(['hg', '-q', 'up', '-R', tempdir, '-r', 'default'])
 
     path = os.path.join(tempdir, 'malwaredomains_full.txt')
     file = codecs.open(path, 'wb', encoding='utf-8')
 
-    print >>file, filterlist_header
+    print >>file, FILTERLIST_HEADER
 
     for mirror in mirrors_list:
-      data = try_mirror(mirror, malwaredomains_path)
+      data = try_mirror(mirror, MALWAREDOMAINS_PATH)

[Sebastian Noack, 2016/03/14 15:40:12]

Since MALWAREDOMAINS_PATH is a global anyway, there is IMO no reason to pass it
around.

[Vasily Kuznetsov, 2016/03/14 16:20:38]

Yeah, I was thinking about it. I guess try_mirror is never intended to be used
anywhere other than this module, so it makes sense to just remove its second
argument.

       if data is not None:
         break
     else:
       print >>sys.stderr, 'Unable to fetch malware domains list.'
       sys.exit(1)

[Sebastian Noack, 2016/03/14 15:40:12]

You can just pass the error message to sys.exit() and Python will terminate with
exit code 1, printing that message to stderr.

[Vasily Kuznetsov, 2016/03/14 16:20:38]

Oh, I didn't know this. Done.

 
     zip = zipfile.ZipFile(StringIO(data), 'r')
     info = zip.infolist()[0]