Fix Incident - deprecation warning incorrectly displayed

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## 0.6.2dev
 
+* [Fix] Deprecation warning incorrectly displayed #213
+
 ## 0.6.1 (2023-03-02)
 
 * [Feature] Support new variable substitution using `{{variable}}` format ([#137](https://github.com/ploomber/jupysql/pull/137))

src/sql/command.py

@@ -1,4 +1,3 @@
-import re
 import warnings
 from IPython.core.magic_arguments import parse_argstring
 from jinja2 import Template
@@ -22,8 +21,6 @@ class SQLCommand:
     """
 
     def __init__(self, magic, user_ns, line, cell) -> None:
-        # Support for the variable substition in the SQL clause
-        line, cell = self._var_expand(magic, user_ns, line, cell)
         self.args = parse.magic_args(magic.execute, line)
         # self.args.line (everything that appears after %sql/%%sql in the first line)
         # is splited in tokens (delimited by spaces), this checks if we have one arg
@@ -53,7 +50,9 @@ def __init__(self, magic, user_ns, line, cell) -> None:
 
         self.parsed = parse.parse(self.command_text, magic)
 
-        self.parsed["sql_original"] = self.parsed["sql"]
+        self.parsed["sql_original"] = self.parsed["sql"] = self._var_expand(
+            self.parsed["sql"], user_ns, magic
+        )
 
         if add_conn:
             self.parsed["connection"] = user_ns[self.args.line[0]]
@@ -89,32 +88,19 @@ def result_var(self):
         """Returns the result_var"""
         return self.parsed["result_var"]
 
-    def _var_expand(self, magic, user_ns, line, cell):
-        """
-        Support for the variable substition in the SQL clause
-        For now, we have enabled two ways:
-        1. Latest format, {{a}}, we use jinja2 to parse the string with {{a}} format
-        2. Legacy format, {a}, $a, and :a format.
+    def _var_expand(self, sql, user_ns, magic):
+        sql = Template(sql).render(user_ns)
+        parsed_sql = magic.shell.var_expand(sql, depth=2)
 
-        We will deprecate the legacy format feature in next major version
-        """
-        self.is_legacy_var_expand_parsed = False
-        # Latest format parsing
-        # TODO: support --param and --use-global logic here
-        # Ref: https://github.com/ploomber/jupysql/issues/93
-        line = Template(line).render(user_ns)
-        cell = Template(cell).render(user_ns)
-        # Legacy format parsing
-        parsed_cell = magic.shell.var_expand(cell, depth=2)
-        parsed_line = magic.shell.var_expand(line, depth=2)
-        # Exclusive the string with "://", but has :variable
-        has_SQLAlchemy_var_expand = re.search("(?<!://):[^/]+", line) or ":" in cell
-        if parsed_line != line or parsed_cell != cell or has_SQLAlchemy_var_expand:
-            self.is_legacy_var_expand_parsed = True
+        has_SQLAlchemy_var_expand = ":" in sql
+        # parsed_sql != sql: detect if using IPython fashion - {a} or $a
+        # has_SQLAlchemy_var_expand: detect if using Sqlalchemy fashion - :a
+        if parsed_sql != sql or has_SQLAlchemy_var_expand:
             warnings.warn(
                 "Variable substitution with $var and {var} has been "
                 "deprecated and will be removed in a future version. "
                 "Use {{var}} instead.",
                 FutureWarning,
             )
-        return parsed_line, parsed_cell
+
+        return parsed_sql

src/tests/integration/test_generic_db_opeations.py

@@ -1,5 +1,6 @@
 import shutil
 import pytest
+import warnings
 from sql.telemetry import telemetry
 from unittest.mock import ANY, Mock
 
@@ -106,8 +107,11 @@ def test_close_and_connect(
     # Disconnect
     ip_with_dynamic_db.run_cell("%sql -x " + conn_alias)
     assert get_connection_count(ip_with_dynamic_db) == 0
-    # Connect
-    ip_with_dynamic_db.run_cell("%sql " + database_url + " --alias " + conn_alias)
+    # Connect, also check there is no error on re-connecting
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        ip_with_dynamic_db.run_cell("%sql " + database_url + " --alias " + conn_alias)
+
     assert get_connection_count(ip_with_dynamic_db) == 1
 
 

src/tests/test_command.py

@@ -14,11 +14,11 @@ def sql_magic(ip):
 @pytest.mark.parametrize(
     "line, cell, parsed_sql, parsed_connection, parsed_result_var",
     [
-        ("something --no-execute", "", "something\n", "", None),
+        ("something --no-execute", "", "something", "", None),
         ("sqlite://", "", "", "sqlite://", None),
-        ("SELECT * FROM TABLE", "", "SELECT * FROM TABLE\n", "", None),
+        ("SELECT * FROM TABLE", "", "SELECT * FROM TABLE", "", None),
         ("SELECT * FROM", "TABLE", "SELECT * FROM\nTABLE", "", None),
-        ("my_var << SELECT * FROM table", "", "SELECT * FROM table\n", "", "my_var"),
+        ("my_var << SELECT * FROM table", "", "SELECT * FROM table", "", "my_var"),
         ("my_var << SELECT *", "FROM table", "SELECT *\nFROM table", "", "my_var"),
         ("[db]", "", "", "sqlite://", None),
     ],
@@ -104,13 +104,13 @@ def test_parsed_sql_when_using_file(ip, sql_magic, tmp_empty):
     assert cmd.parsed == {
         "connection": "",
         "result_var": None,
-        "sql": "SELECT * FROM author\n\n",
-        "sql_original": "SELECT * FROM author\n\n",
+        "sql": "SELECT * FROM author\n",
+        "sql_original": "SELECT * FROM author\n",
     }
 
     assert cmd.connection == ""
-    assert cmd.sql == "SELECT * FROM author\n\n"
-    assert cmd.sql_original == "SELECT * FROM author\n\n"
+    assert cmd.sql == "SELECT * FROM author\n"
+    assert cmd.sql_original == "SELECT * FROM author\n"
 
 
 def test_args(ip, sql_magic):
@@ -220,7 +220,7 @@ def test_variable_substitution_legacy_dollar_prefix_cell_magic(ip, sql_magic):
         cell="GRANT CONNECT ON DATABASE postgres TO $username;",
     )
 
-    assert cmd.parsed["sql"] == "\nGRANT CONNECT ON DATABASE postgres TO some-user;"
+    assert cmd.parsed["sql"] == "GRANT CONNECT ON DATABASE postgres TO some-user;"
 
 
 def test_variable_substitution_legacy_single_curly_cell_magic(ip, sql_magic):
@@ -261,4 +261,4 @@ def test_variable_substitution_double_curly_line_magic(ip, sql_magic):
     )
 
     # print ("cmd.parsed['sql']", cmd.parsed["sql"])
-    assert cmd.parsed["sql"] == "SELECT first_name FROM author LIMIT 5;\n"
+    assert cmd.parsed["sql"] == "SELECT first_name FROM author LIMIT 5;"