Merge pull request pallets-eco#731 from lbeaufort/663-add-error-missing-uri-no-binds

Default SQLALCHEMY_DATABASE_URI to None, error if no URI or binds

Author: davidism

CHANGES.rst

@@ -6,8 +6,11 @@ Unreleased
 -   Drop support for Python 3.4.
 -   Bump minimum version of Flask to 1.0.4.
 -   Bump minimum version of SQLAlchemy to 1.2.
--   Set `SQLALCHEMY_TRACK_MODIFICATIONS` to `False` by default,
+-   Set ``SQLALCHEMY_TRACK_MODIFICATIONS`` to ``False`` by default,
     remove deprecation warning (:pr:`727`)
+-   Remove default ``'sqlite:///:memory:'`` setting for
+    ``SQLALCHEMY_DATABASE_URI``, raise error when both ``SQLALCHEMY_DATABASE_URI``
+    and ``SQLALCHEMY_BINDS`` are unset (:pr:`731`)
 
 
 Version 2.4.1

docs/config.rst

@@ -106,6 +106,10 @@ A list of configuration keys currently understood by the extension:
   * ``SQLALCHEMY_POOL_RECYCLE``
   * ``SQLALCHEMY_MAX_OVERFLOW``
 
+.. versionchanged:: 3.0
+
+  * ``SQLALCHEMY_TRACK_MODIFICATIONS`` configuration key now defaults to ``False``
+  * ``SQLALCHEMY_DATABASE_URI``  configuration key no longer defaults to ``'sqlite:///:memory:'``
 
 Connection URI Format
 ---------------------

flask_sqlalchemy/__init__.py

@@ -810,19 +810,18 @@ def init_app(self, app):
         of an application not initialized that way or connections will
         leak.
         """
+
         # We intentionally don't set self.app = app, to support multiple
         # applications. If the app is passed in the constructor,
         # we set it and don't support multiple applications.
-        if (
-            'SQLALCHEMY_DATABASE_URI' not in app.config and
-            'SQLALCHEMY_BINDS' not in app.config
+        if not (
+            app.config.get('SQLALCHEMY_DATABASE_URI')
+            or app.config.get('SQLALCHEMY_BINDS')
         ):
-            warnings.warn(
-                'Neither SQLALCHEMY_DATABASE_URI nor SQLALCHEMY_BINDS is set. '
-                'Defaulting SQLALCHEMY_DATABASE_URI to "sqlite:///:memory:".'
-            )
+            raise RuntimeError('Either SQLALCHEMY_DATABASE_URI '
+                               'or SQLALCHEMY_BINDS needs to be set.')
 
-        app.config.setdefault('SQLALCHEMY_DATABASE_URI', 'sqlite:///:memory:')
+        app.config.setdefault('SQLALCHEMY_DATABASE_URI', None)
         app.config.setdefault('SQLALCHEMY_BINDS', None)
         app.config.setdefault('SQLALCHEMY_NATIVE_UNICODE', None)
         app.config.setdefault('SQLALCHEMY_ECHO', False)

tests/test_config.py

@@ -18,18 +18,39 @@ def app_nr(app):
 
 class TestConfigKeys:
 
-    def test_defaults(self, app, recwarn):
+    def test_default_error_without_uri_or_binds(self, app, recwarn):
         """
-        Test all documented config values in the order they appear in our
-        documentation: http://flask-sqlalchemy.pocoo.org/dev/config/
+        Test that default configuration throws an error because
+        SQLALCHEMY_DATABASE_URI and SQLALCHEMY_BINDS are unset
         """
 
         fsa.SQLAlchemy(app)
 
-        # Expecting no warnings for default config
+        # Our pytest fixture for creating the app sets
+        # SQLALCHEMY_DATABASE_URI, so undo that here so that we
+        # can inspect what FSA does below:
+        del app.config['SQLALCHEMY_DATABASE_URI']
+
+        with pytest.raises(RuntimeError) as exc_info:
+            fsa.SQLAlchemy(app)
+
+        expected = 'Either SQLALCHEMY_DATABASE_URI ' \
+                   'or SQLALCHEMY_BINDS needs to be set.'
+        assert exc_info.value.args[0] == expected
+
+    def test_defaults_with_uri(self, app, recwarn):
+        """
+        Test default config values when URI is provided, in the order they
+        appear in the documentation: http://flask-sqlalchemy.pocoo.org/dev/config/
+
+        Our pytest fixture for creating the app sets SQLALCHEMY_DATABASE_URI
+        """
+
+        fsa.SQLAlchemy(app)
+
+        # Expecting no warnings for default config with URI
         assert len(recwarn) == 0
 
-        assert app.config['SQLALCHEMY_DATABASE_URI'] == 'sqlite:///:memory:'
         assert app.config['SQLALCHEMY_BINDS'] is None
         assert app.config['SQLALCHEMY_ECHO'] is False
         assert app.config['SQLALCHEMY_RECORD_QUERIES'] is None
@@ -41,18 +62,6 @@ def test_defaults(self, app, recwarn):
         assert app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] is False
         assert app.config['SQLALCHEMY_ENGINE_OPTIONS'] == {}
 
-    def test_uri_binds_warning(self, app, recwarn):
-        # Let's trigger the warning
-        del app.config['SQLALCHEMY_DATABASE_URI']
-        fsa.SQLAlchemy(app)
-
-        # and verify it showed up as expected
-        assert len(recwarn) == 1
-        expect = 'Neither SQLALCHEMY_DATABASE_URI nor SQLALCHEMY_BINDS' \
-                 ' is set. Defaulting SQLALCHEMY_DATABASE_URI to'\
-                 ' "sqlite:///:memory:".'
-        assert recwarn[0].message.args[0] == expect
-
     def test_engine_creation_ok(self, app, recwarn):
         """ create_engine() isn't called until needed.  Let's make sure we can do that without
             errors or warnings.