Fix reading of bucket name and acl settings for S3

CHANGELOG.txt

@@ -1,3 +1,10 @@
+0.1.0 (unreleased)
+==================
+
+- Bucket name is now read from ``storage.aws.bucket_name`` setting, as stated
+  in documentation.
+- ACL is now read from ``storage.aws.acl`` setting, as stated in documentation.
+
 0.0.8 (2014-10-1)
 ==================
 

docs/Makefile

@@ -15,76 +15,76 @@ ALLSPHINXOPTS = -d _build/doctrees $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) .
 .PHONY: help clean html web pickle htmlhelp latex changes linkcheck
 
 help:
-        @echo "Please use \`make <target>' where <target> is one of"
-        @echo " html to make standalone HTML files"
-        @echo " pickle to make pickle files (usable by e.g. sphinx-web)"
-        @echo " htmlhelp to make HTML files and a HTML help project"
-        @echo " latex to make LaTeX files, you can set PAPER=a4 or PAPER=letter"
-        @echo " changes to make an overview over all changed/added/deprecated items"
-        @echo " linkcheck to check all external links for integrity"
+	@echo "Please use 'make <target>' where <target> is one of"
+	@echo " html to make standalone HTML files"
+	@echo " pickle to make pickle files (usable by e.g. sphinx-web)"
+	@echo " htmlhelp to make HTML files and a HTML help project"
+	@echo " latex to make LaTeX files, you can set PAPER=a4 or PAPER=letter"
+	@echo " changes to make an overview over all changed/added/deprecated items"
+	@echo " linkcheck to check all external links for integrity"
 
 clean:
-        -rm -rf _build/*
+	-rm -rf _build/*
 
 html: _themes
-        mkdir -p _build/html _build/doctrees
-        $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) _build/html
-        @echo
-        @echo "Build finished. The HTML pages are in _build/html."
+	mkdir -p _build/html _build/doctrees
+	$(SPHINXBUILD) -b html $(ALLSPHINXOPTS) _build/html
+	@echo
+	@echo "Build finished. The HTML pages are in _build/html."
 
 text:
-        mkdir -p _build/text _build/doctrees
-        $(SPHINXBUILD) -b text $(ALLSPHINXOPTS) _build/text
-        @echo
-        @echo "Build finished. The HTML pages are in _build/text."
+	mkdir -p _build/text _build/doctrees
+	$(SPHINXBUILD) -b text $(ALLSPHINXOPTS) _build/text
+	@echo
+	@echo "Build finished. The HTML pages are in _build/text."
 
 pickle:
-        mkdir -p _build/pickle _build/doctrees
-        $(SPHINXBUILD) -b pickle $(ALLSPHINXOPTS) _build/pickle
-        @echo
-        @echo "Build finished; now you can process the pickle files or run"
-        @echo " sphinx-web _build/pickle"
-        @echo "to start the sphinx-web server."
+	mkdir -p _build/pickle _build/doctrees
+	$(SPHINXBUILD) -b pickle $(ALLSPHINXOPTS) _build/pickle
+	@echo
+	@echo "Build finished; now you can process the pickle files or run"
+	@echo " sphinx-web _build/pickle"
+	@echo "to start the sphinx-web server."
 
 web: pickle
 
 htmlhelp: _themes
-        mkdir -p _build/htmlhelp _build/doctrees
-        $(SPHINXBUILD) -b htmlhelp $(ALLSPHINXOPTS) _build/htmlhelp
-        @echo
-        @echo "Build finished; now you can run HTML Help Workshop with the" \
-         ".hhp project file in _build/htmlhelp."
+	mkdir -p _build/htmlhelp _build/doctrees
+	$(SPHINXBUILD) -b htmlhelp $(ALLSPHINXOPTS) _build/htmlhelp
+	@echo
+	@echo "Build finished; now you can run HTML Help Workshop with the" \
+	 ".hhp project file in _build/htmlhelp."
 
 latex:
-        mkdir -p _build/latex _build/doctrees
-        $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) _build/latex
-        cp _static/*.png _build/latex
-        ./convert_images.sh
-        cp _static/latex-warning.png _build/latex
-        cp _static/latex-note.png _build/latex
-        @echo
-        @echo "Build finished; the LaTeX files are in _build/latex."
-        @echo "Run \`make all-pdf' or \`make all-ps' in that directory to" \
-         "run these through (pdf)latex."
+	mkdir -p _build/latex _build/doctrees
+	$(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) _build/latex
+	cp _static/*.png _build/latex
+	./convert_images.sh
+	cp _static/latex-warning.png _build/latex
+	cp _static/latex-note.png _build/latex
+	@echo
+	@echo "Build finished; the LaTeX files are in _build/latex."
+	@echo "Run \`make all-pdf' or \`make all-ps' in that directory to" \
+	 "run these through (pdf)latex."
 
 changes:
-        mkdir -p _build/changes _build/doctrees
-        $(SPHINXBUILD) -b changes $(ALLSPHINXOPTS) _build/changes
-        @echo
-        @echo "The overview file is in _build/changes."
+	mkdir -p _build/changes _build/doctrees
+	$(SPHINXBUILD) -b changes $(ALLSPHINXOPTS) _build/changes
+	@echo
+	@echo "The overview file is in _build/changes."
 
 linkcheck:
-        mkdir -p _build/linkcheck _build/doctrees
-        $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) _build/linkcheck
-        @echo
-        @echo "Link check complete; look for any errors in the above output " \
-         "or in _build/linkcheck/output.txt."
+	mkdir -p _build/linkcheck _build/doctrees
+	$(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) _build/linkcheck
+	@echo
+	@echo "Link check complete; look for any errors in the above output " \
+	 "or in _build/linkcheck/output.txt."
 
 epub:
-        $(SPHINXBUILD) -b epub $(ALLSPHINXOPTS) _build/epub
-        @echo
-        @echo "Build finished. The epub file is in _build/epub."
+	$(SPHINXBUILD) -b epub $(ALLSPHINXOPTS) _build/epub
+	@echo
+	@echo "Build finished. The epub file is in _build/epub."
 
 _themes:
-        cd ..; git submodule update --init; cd docs
+	cd ..; git submodule update --init; cd docs
 

docs/index.rst

@@ -21,7 +21,7 @@ There are a number of ways to configure **pyramid_storage** with your Pyramid ap
 
 The easiest way is to add **pyramid_storage** to the **pyramid.includes** directive in your configuration file(s)::
 
-    pyramid.includes = 
+    pyramid.includes =
         pyramid_storage
 
 
@@ -35,7 +35,7 @@ will also be available as a property of your request as **request.storage**.
 
 To use S3 file storage instead of storing files locally on your server (the default assumption)::
 
-    pyramid.includes = 
+    pyramid.includes =
         pyramid_storage.s3
 
 alternatively::
@@ -67,7 +67,7 @@ Setting                Default                Description
 **aws.access_key**     **required**           AWS access key
 **aws.secret_key**     **required**           AWS secret key
 **aws.bucket_name**    **required**           AWS bucket
-**aws.acl**            ``public-read``        AWS ACL permissions
+**aws.acl**            ``public-read``        `AWS ACL permissions <https://github.com/boto/boto/blob/v2.13.2/boto/s3/acl.py#L25-L28>`_
 **base_url**                                  Relative or absolute base URL for uploads; must end in slash ("/")
 **extensions**         ``default``            List of extensions or extension groups (see below)
 **name**               ``storage``            Name of property added to request, e.g. **request.storage**
@@ -120,7 +120,7 @@ When uploading a file in a view, call :meth:`pyramid_storage.storage.FileStorage
         return HTTPSeeOther(request.route_url('home'))
 
 
-This operation will save the file to your file system under the top directory specified by the **base_path** setting. 
+This operation will save the file to your file system under the top directory specified by the **base_path** setting.
 
 If the file does not have the correct file extension, a :class:`pyramid_storage.exceptions.FileNotAllowed` exception is raised. A more secure way of writing the above would be::
 
@@ -144,7 +144,7 @@ You can override the default extensions in the method call::
                  request_method='POST')
     def upload(request):
         try:
-            request.storage.save(request.POST['my_file'],  
+            request.storage.save(request.POST['my_file'],
                                  extensions=('jpg', 'png', 'txt'))
         except FileNotAllowed:
             request.session.flash('Sorry, this file is not allowed')
@@ -157,7 +157,7 @@ You may also wish to obfuscate or randomize the filename. The ``randomize`` argu
 
 So for example if your filename is ``test.jpg`` the new filename will be something like ``235a344c-8d70-498a-af0a-151afdfcd803.jpg``.
 
-If there is a filename clash (i.e. another file with the same name is in the target directory) a numerical suffix is added to the new filename. For example, 
+If there is a filename clash (i.e. another file with the same name is in the target directory) a numerical suffix is added to the new filename. For example,
 if you have an existing file ``test.jpg`` then the next file with that name will be renamed ``test-1.jpg`` and so on.
 
 .. warning::

pyramid_storage/local.py

@@ -45,17 +45,7 @@ def from_settings(cls, settings, prefix):
             ('base_url', False, ''),
             ('extensions', False, 'default'),
         )
-
-        kwargs = {}
-
-        for name, required, default in options:
-            try:
-                kwargs[name] = settings[prefix + name]
-            except KeyError:
-                if required:
-                    raise ValueError("%s%s is required" % (prefix, name))
-                kwargs[name] = default
-
+        kwargs = utils.read_settings(settings, options, prefix)
         return cls(**kwargs)
 
     def __init__(self, base_path, base_url='', extensions='default'):

pyramid_storage/s3.py

@@ -27,12 +27,17 @@ class S3FileStorage(object):
 
     @classmethod
     def from_settings(cls, settings, prefix):
-        return cls(access_key=settings[prefix + 'aws.access_key'],
-                   secret_key=settings[prefix + 'aws.secret_key'],
-                   bucket_name=settings[prefix + 'aws.bucket'],
-                   acl=settings.get(prefix + 'aws.default_acl', 'public-read'),
-                   base_url=settings.get(prefix + 'base_url', ''),
-                   extensions=settings.get(prefix + 'extensions', 'default'))
+        options = (
+            ('aws.access_key', True, None),
+            ('aws.secret_key', True, ''),
+            ('aws.bucket_name', True, None),
+            ('aws.acl', False, 'public-read'),
+            ('base_url', False, ''),
+            ('extensions', False, 'default'),
+        )
+        kwargs = utils.read_settings(settings, options, prefix)
+        kwargs = dict([(k.replace('aws.', ''), v) for k, v in kwargs.items()])
+        return cls(**kwargs)
 
     def __init__(self, access_key, secret_key, bucket_name,
                  acl=None, base_url='', extensions='default'):

pyramid_storage/utils.py

@@ -6,6 +6,7 @@
 import unicodedata
 
 from pyramid import compat
+from pyramid import exceptions as pyramid_exceptions
 
 
 _filename_ascii_strip_re = re.compile(r'[^A-Za-z0-9_.-]')
@@ -49,3 +50,27 @@ def random_filename(filename):
     """
     _, ext = os.path.splitext(filename)
     return str(uuid.uuid4()) + ext.lower()
+
+
+def read_settings(settings, options, prefix=''):
+    """Reads the `settings` dictionnary, and sets defaults using the
+    provided list of tuples in `options`.
+
+    :param settings: settings to read.
+    :param options: a list of tuples (name, required, default).
+    :param prefix: prefix for the settings keys.
+    :returns: a dictionnary with defaults set.
+    :raises: :exc:`~pyramid:pyramid.exceptions.ConfigurationError` if a
+        required setting is not provided.
+    """
+    result = {}
+    for name, required, default in options:
+        setting = prefix + name
+        try:
+            result[name] = settings[setting]
+        except KeyError:
+            if required:
+                error_msg = "%s is required" % setting
+                raise pyramid_exceptions.ConfigurationError(error_msg)
+            result[name] = default
+    return result

tests/test_local.py

@@ -5,6 +5,7 @@
 import pytest
 
 from pyramid import compat
+from pyramid import exceptions as pyramid_exceptions
 
 
 def _mock_open_name():
@@ -282,5 +283,5 @@ def test_from_settings_if_base_path_missing():
 
     from pyramid_storage import local
 
-    with pytest.raises(ValueError):
+    with pytest.raises(pyramid_exceptions.ConfigurationError):
         local.LocalFileStorage.from_settings({}, 'storage.')

tests/test_s3.py

@@ -4,6 +4,7 @@
 import pytest
 
 from pyramid import compat
+from pyramid import exceptions as pyramid_exceptions
 
 
 class MockS3Connection(object):
@@ -226,3 +227,29 @@ def test_delete():
             _get_mock_s3_connection):
 
         s.delete("test.jpg")
+
+
+def test_from_settings_with_defaults():
+
+    from pyramid_storage import s3
+
+    settings = {
+        'storage.aws.access_key': 'abc',
+        'storage.aws.secret_key': '123',
+        'storage.aws.bucket_name': 'Attachments',
+    }
+    inst = s3.S3FileStorage.from_settings(settings, 'storage.')
+    assert inst.base_url == ''
+    assert inst.access_key == 'abc'
+    assert inst.secret_key == '123'
+    assert inst.bucket_name == 'Attachments'
+    assert inst.acl == 'public-read'
+    assert set(('jpg', 'txt', 'doc')).intersection(inst.extensions)
+
+
+def test_from_settings_if_base_path_missing():
+
+    from pyramid_storage import s3
+
+    with pytest.raises(pyramid_exceptions.ConfigurationError):
+        s3.S3FileStorage.from_settings({}, 'storage.')