Issue 2340 - Don`t generate pages if less than 30% have been translated

cms/bin/generate_static_pages.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 sys
 import os
 import re
 import errno
 import codecs
 import ConfigParser
-import functools
 import logging
 
 from cms.utils import get_page_params, process_page
 from cms.sources import MercurialSource
 
 MIN_TRANSLATED = 0.3
 
 def memoize(func):
   memoized = {}
   def wrapper(*args):
     try:
       return memoized[args]
     except KeyError:
       return memoized.setdefault(args, func(*args))
-  wrapper.original = func

[Sebastian Noack, 2015/05/06 16:15:30]

I'm not sure if I like that hack. Note that it wouldn't be neccessarry if you
would pass the dict object used for caching as an argument:

  cached_links = {}
  source.resolve_link = memoize(source.resolve_link, cached_links)

  ...

  cached_links.clear()

[Wladimir Palant, 2015/05/06 17:21:59]

True, I don't like passing in a dictionary either however. Given that it can
only really be used to clear the cache, I can just expose that functionality.

[Sebastian Noack, 2015/05/06 19:12:15]

Well, allowing to pass in a dict-like object, also helps other use cases, like
implementing different kind of caches (like and LRU cache or using an external
cache backend). Or to share a cache for different functions. It's actually quite
common practice to take a cache argument when implementing a memoize() function,
django for example does that.

It also has the advantage that you might not mistakenly call .clear_cache() on a
function that wasn't actually decorated with memoize(). In fact it took my a
while to figure out where the "original" argument came from.

Also note that it's common practice, at least for decorators that are supposed
to work with any callable object, to use functools.update_wrapper() to copy
attribute from the decorated function to wrapper. That's not necessary here, but
if this decorator becomes more widely used, we might want to do that. And adding
additional attributes can cause conflicts then.

However, I'm not feeling too strong about it. So LGTM if you disagree.

[Wladimir Palant, 2015/05/06 19:20:35]

Thank you. Indeed, I do feel that the current approach is better than just
manipulating the dictionary used by memoize() and potentially breaking
assumptions that this decorator makes.

+  wrapper.clear_cache = memoized.clear
   return wrapper
 
 def generate_pages(repo, output_dir):
   known_files = set()
 
   def write_file(path_parts, contents, binary=False):
     encoding = None if binary else "utf-8"
     outfile = os.path.join(output_dir, *path_parts)
     if outfile in known_files:
       logging.warning("File %s has multiple sources", outfile)
@@ skipping 24 matching lines @@
     source.read_include = memoize(source.read_include)
 
     config = source.read_config()
     defaultlocale = config.get("general", "defaultlocale")
     locales = list(source.list_locales())
     if defaultlocale not in locales:
       locales.append(defaultlocale)
 
     # First pass: compile the list of pages with given translation level
     pagelist = set()
+    blacklist = set()
     for page, format in source.list_pages():
       for locale in locales:
         if locale == defaultlocale:
           pagelist.add((locale, page))
         elif source.has_locale(locale, page):
           params = get_page_params(source, locale, page, format)
           if params["translation_ratio"] >= MIN_TRANSLATED:
             pagelist.add((locale, page))
+          else:
+            blacklist.add((locale, page))
 
     # Override existance check to avoid linking to pages we don't generate
-    def has_locale(orig_function, locale, page):
+    orig_has_locale = source.has_locale
+    def has_locale(locale, page):
       try:
         page = config.get("locale_overrides", page)
       except ConfigParser.Error:
         pass
-      if (defaultlocale, page) in pagelist:
-        return locale != defaultlocale and (locale, page) in pagelist
-      else:

[Sebastian Noack, 2015/05/06 16:15:30]

Nit: Redundant else statement. (Yes, I know you like them)

[Wladimir Palant, 2015/05/06 17:21:59]

Done and made the logic here slightly more obvious.

-        return orig_function(locale, page)
-    source.has_locale = functools.partial(has_locale, source.has_locale)

[Sebastian Noack, 2015/05/06 16:15:30]

It took me a while to understand what you are doing here. Would be more clear
IMO, if you just use a closure variable:

  orig_has_locale = source.has_locale
  def has_locale(locale, page):
    ..
    return orig_has_locale(locale, page)
  source.has_locale = has_locale

[Wladimir Palant, 2015/05/06 17:21:59]

I thought you were a fan of clever solutions :)

-    source.resolve_link = memoize(source.resolve_link.original)
+      if (locale, page) in blacklist:
+        return False
+      return orig_has_locale(locale, page)
+    source.has_locale = has_locale
+    source.resolve_link.clear_cache()
 
     # Second pass: actually generate pages this time
     for locale, page in pagelist:
       pagedata = process_page(source, locale, page)
 
       # Make sure links to static files are versioned
       pagedata = re.sub(r'(<script\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
       pagedata = re.sub(r'(<link\s[^<>]*\bhref="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
       pagedata = re.sub(r'(<img\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
 
@@ skipping 20 matching lines @@
           os.rmdir(path)
   remove_unknown(output_dir)
 
 if __name__ == "__main__":
   if len(sys.argv) < 3:
     print >>sys.stderr, "Usage: %s source_repository output_dir" % sys.argv[0]
     sys.exit(1)
 
   repo, output_dir = sys.argv[1:3]
   generate_pages(repo, output_dir)