Repository: striglia/pyramid_swagger (python)

Issue:
Improve how pyramid_swagger builds bravado-core configs
The current implementation of [``create_bravado_core_config``](https://github.com/striglia/pyramid_swagger/blob/c97c227f2a103b73fbf72906cac977df114ad622/pyramid_swagger/ingest.py#L188) limits the bravado-core configurations that could be set. The project already had some issues asking to expose new bravado-core configs through pyramid_swagger configs (ie. #211).

I'm wondering if we could actually have a passthrough prefix for setting bravado-core configs.
The idea is to use all the pyramid_swagger configs that starts with a given prefix as bravado-core configs like ``bravado_core.XXX: 1`` will set bravado-core config XXX to 1


Pull request:
bravado-core passthrough configs
Fixes #220.

This PR deprecates (with a warning only for now) the usage of ``pyramid_swagger.xxx`` configs for the configuration of the underlying ``bravado_core`` instance.
In order to _trick_ bravado-core related configs ``bravado_core.xxx`` should be used instead.

Patch:
diff --git a/docs/configuration.rst b/docs/configuration.rst
index c6357ea..e32aa92 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -97,6 +97,12 @@ A few relevant settings for your `Pyramid .ini file <http://docs.pylonsproject.o
         pyramid_swagger.dereference_served_schema = false
 
 
+.. note::
+
+    ``pyramid_swawgger`` uses a ``bravado_core.spec.Spec`` instance for handling swagger related details.
+    You can set `bravado-core config values <http://bravado-core.readthedocs.io/en/stable/config.html>`_ by adding a ``bravado-core.`` prefix to them.
+
+
 Note that, equivalently, you can add these settings during webapp configuration:
 
 .. code-block:: python
diff --git a/pyramid_swagger/ingest.py b/pyramid_swagger/ingest.py
index cfe3b7d..4729fa1 100644
--- a/pyramid_swagger/ingest.py
+++ b/pyramid_swagger/ingest.py
@@ -3,10 +3,12 @@ from __future__ import unicode_literals
 
 import glob
 import os.path
+import warnings
 
 import simplejson
 from bravado_core.spec import build_http_handlers
 from bravado_core.spec import Spec
+from six import iteritems
 from six.moves.urllib import parse as urlparse
 
 from .api import build_swagger_12_endpoints
@@ -16,6 +18,10 @@ from .spec import API_DOCS_FILENAME
 from .spec import validate_swagger_schema
 
 
+# Prefix of configs that will be passed to the underlying bravado-core instance
+BRAVADO_CORE_CONFIG_PREFIX = 'bravado_core.'
+
+
 class ResourceListingNotFoundError(Exception):
     pass
 
@@ -200,21 +206,41 @@ def create_bravado_core_config(settings):
     config_keys = {
         'pyramid_swagger.enable_request_validation': 'validate_requests',
         'pyramid_swagger.enable_response_validation': 'validate_responses',
-        'pyramid_swagger.enable_swagger_spec_validation':
-            'validate_swagger_spec',
+        'pyramid_swagger.enable_swagger_spec_validation': 'validate_swagger_spec',
         'pyramid_swagger.use_models': 'use_models',
         'pyramid_swagger.user_formats': 'formats',
         'pyramid_swagger.include_missing_properties': 'include_missing_properties',
     }
 
-    bravado_core_config_defaults = {
+    configs = {
         'use_models': False
     }
 
-    return dict(bravado_core_config_defaults, **dict(
-        (bravado_core_key, settings[pyramid_swagger_key])
-        for pyramid_swagger_key, bravado_core_key in config_keys.items()
-        if pyramid_swagger_key in settings))
+    bravado_core_configs_from_pyramid_swagger_configs = {
+        bravado_core_key: settings[pyramid_swagger_key]
+        for pyramid_swagger_key, bravado_core_key in iteritems(config_keys)
+        if pyramid_swagger_key in settings
+    }
+    if bravado_core_configs_from_pyramid_swagger_configs:
+        warnings.warn(
+            message='Configs {old_configs} are deprecated, please use {new_configs} instead.'.format(
+                old_configs=', '.join(k for k, v in sorted(iteritems(config_keys))),
+                new_configs=', '.join(
+                    '{}{}'.format(BRAVADO_CORE_CONFIG_PREFIX, v)
+                    for k, v in sorted(iteritems(config_keys))
+                ),
+            ),
+            category=DeprecationWarning,
+        )
+        configs.update(bravado_core_configs_from_pyramid_swagger_configs)
+
+    configs.update({
+        key.replace(BRAVADO_CORE_CONFIG_PREFIX, ''): value
+        for key, value in iteritems(settings)
+        if key.startswith(BRAVADO_CORE_CONFIG_PREFIX)
+    })
+
+    return configs
 
 
 def ingest_resources(mapping, schema_dir):
@@ -228,7 +254,7 @@ def ingest_resources(mapping, schema_dir):
         :class:`ValidatorMap`
     """
     ingested_resources = []
-    for name, filepath in mapping.items():
+    for name, filepath in iteritems(mapping):
         try:
             ingested_resources.append(load_schema(filepath))
         # If we have trouble reading any files, raise a more user-friendly


Test patch:
diff --git a/tests/ingest_test.py b/tests/ingest_test.py
index a06950d..9f6dc93 100644
--- a/tests/ingest_test.py
+++ b/tests/ingest_test.py
@@ -8,6 +8,7 @@ import simplejson
 from pyramid_swagger.ingest import _load_resource_listing
 from pyramid_swagger.ingest import API_DOCS_FILENAME
 from pyramid_swagger.ingest import ApiDeclarationNotFoundError
+from pyramid_swagger.ingest import BRAVADO_CORE_CONFIG_PREFIX
 from pyramid_swagger.ingest import create_bravado_core_config
 from pyramid_swagger.ingest import generate_resource_listing
 from pyramid_swagger.ingest import get_resource_listing
@@ -120,23 +121,65 @@ def test_create_bravado_core_config_with_defaults():
     assert {'use_models': False} == create_bravado_core_config({})
 
 
-def test_create_bravado_core_config_non_empty():
-    some_format = mock.Mock(spec=SwaggerFormat)
+@pytest.fixture
+def bravado_core_formats():
+    return [mock.Mock(spec=SwaggerFormat)]
+
+
+@pytest.fixture
+def bravado_core_configs(bravado_core_formats):
+    return {
+        'validate_requests': True,
+        'validate_responses': False,
+        'validate_swagger_spec': True,
+        'use_models': True,
+        'formats': bravado_core_formats,
+        'include_missing_properties': False
+    }
+
+
+@mock.patch('pyramid_swagger.ingest.warnings')
+def test_create_bravado_core_config_non_empty_deprecated_configs(mock_warnings, bravado_core_formats, bravado_core_configs):
     pyramid_swagger_config = {
         'pyramid_swagger.enable_request_validation': True,
         'pyramid_swagger.enable_response_validation': False,
         'pyramid_swagger.enable_swagger_spec_validation': True,
         'pyramid_swagger.use_models': True,
-        'pyramid_swagger.user_formats': [some_format],
+        'pyramid_swagger.user_formats': bravado_core_formats,
         'pyramid_swagger.include_missing_properties': False,
     }
-    expected_bravado_core_config = {
-        'validate_requests': True,
-        'validate_responses': False,
-        'validate_swagger_spec': True,
-        'use_models': True,
-        'formats': [some_format],
-        'include_missing_properties': False
+
+    bravado_core_config = create_bravado_core_config(pyramid_swagger_config)
+
+    assert bravado_core_configs == bravado_core_config
+    # NOTE: the assertion is detailed and not defined by a constant because PYRAMID_SWAGGER_TO_BRAVADO_CORE_CONFIGS_MAPPING
+    # usage is deprecated and will eventually be removed in future versions
+    mock_warnings.warn.assert_called_once_with(
+        message='Configs '
+                'pyramid_swagger.enable_request_validation, pyramid_swagger.enable_response_validation, '
+                'pyramid_swagger.enable_swagger_spec_validation, pyramid_swagger.include_missing_properties, '
+                'pyramid_swagger.use_models, pyramid_swagger.user_formats '
+                'are deprecated, please use '
+                'bravado_core.validate_requests, bravado_core.validate_responses, '
+                'bravado_core.validate_swagger_spec, bravado_core.include_missing_properties, '
+                'bravado_core.use_models, bravado_core.formats '
+                'instead.',
+        category=DeprecationWarning,
+    )
+
+
+@mock.patch('pyramid_swagger.ingest.warnings')
+def test_create_bravado_core_config_with_passthrough_configs(mock_warnings, bravado_core_formats, bravado_core_configs):
+    pyramid_swagger_config = {
+        '{}validate_requests'.format(BRAVADO_CORE_CONFIG_PREFIX): True,
+        '{}validate_responses'.format(BRAVADO_CORE_CONFIG_PREFIX): False,
+        '{}validate_swagger_spec'.format(BRAVADO_CORE_CONFIG_PREFIX): True,
+        '{}use_models'.format(BRAVADO_CORE_CONFIG_PREFIX): True,
+        '{}formats'.format(BRAVADO_CORE_CONFIG_PREFIX): bravado_core_formats,
+        '{}include_missing_properties'.format(BRAVADO_CORE_CONFIG_PREFIX): False
     }
+
     bravado_core_config = create_bravado_core_config(pyramid_swagger_config)
-    assert expected_bravado_core_config == bravado_core_config
+
+    assert bravado_core_configs == bravado_core_config
+    assert not mock_warnings.warn.called