Assorted improvements.

- Do not use a property for a local variable.
- Use new-style class.
- Stop using logger.
- Save a few getattr calls.
- Do not keep references to buildout & options outside of __init__ .
- Convert mode in __init__ to detect errors early.
- Factorise several accesses (buildout['buildout'], properties)
- Drop redundant "cache" download.Download parameter: it is automatically
  fetched from first parameter internally.
- "Backward compatibility with other recipes" is just not "backward".
- Return downloaded file path, not just the optionally-created directory.

slapos/recipe/download.py

@@ -25,51 +25,48 @@
 #
 ##############################################################################
 import os
-import logging
 import shutil
 import zc.buildout
-class Recipe:
+
+class Recipe(object):
+  _parts = None
+
   def __init__(self, buildout, name, options):
-    self.buildout = buildout
-    self.name = name
-    self.options = options
-    self.logger = logging.getLogger(self.name)
-    if 'filename' in self.options and 'destination' in self.options:
+    buildout_section = buildout['buildout']
+    self._downloader = zc.buildout.download.Download(buildout_section,
+      hash_name=True)
+    self._url = options['url']
+    self._md5sum = options.get('md5sum')
+    mode = options.get('mode')
+    if mode is not None:
+      mode = int(mode, 8)
+    self._mode = mode
+    if 'filename' in options and 'destination' in options:
       raise zc.buildout.UserError('Parameters filename and destination are '
         'exclusive.')
-    self.parts = None
-    self.destination = self.options.get('destination', None)
-    if self.destination is None:
-      self.parts = os.path.join(self.buildout['buildout']['parts-directory'],
-          self.name)
-      self.destination = os.path.join(self.parts, self.options.get('filename',
-        self.name))
-      # backward compatibility with other recipes -- expose location
-      options['location'] = os.path.join(self.buildout['buildout'][
-        'parts-directory'], self.name)
-    options['target'] = self.destination
+    destination = options.get('destination', None)
+    if destination is None:
+      self._parts = parts = os.path.join(buildout_section['parts-directory'],
+        name)
+      destination = os.path.join(parts, options.get('filename', name))
+      # Compatibility with other recipes: expose location
+      options['location'] = parts
+    options['target'] = self._destination = destination
 
   def install(self):
-    if self.parts is not None:
-      if not os.path.isdir(self.parts):
-        os.mkdir(self.parts)
-    download = zc.buildout.download.Download(self.buildout['buildout'],
-        hash_name=True, cache=self.buildout['buildout'].get('download-cache'))
-    path, is_temp = download(self.options['url'],
-        md5sum=self.options.get('md5sum'))
-    if os.path.exists(self.destination):
-      os.unlink(self.destination)
-    shutil.copy(path, self.destination)
-    mode = self.options.get('mode')
+    destination = self._destination
+    result = [destination]
+    parts = self._parts
+    if parts is not None and not os.path.isdir(parts):
+      os.mkdir(parts)
+      result.append(parts)
+    path, _ = self._downloader(self._url, md5sum=self._md5sum)
+    if os.path.exists(destination):
+      os.unlink(destination)
+    shutil.copy(path, destination)
+    mode = self._mode
     if mode is not None:
-      mode = int(mode, 8)
-      os.chmod(self.destination, mode)
-      self.logger.debug('Mode of %r set to 0%o.' % (self.destination, mode))
-    self.logger.debug('Downloaded %r and saved to %r.' % (self.options['url'],
-      self.destination))
-    if self.parts is not None:
-      return [self.parts]
-    else:
-      return []
+      os.chmod(destination, mode)
+    return result
 
   update = install