Repository: ploomber/jupysql (python)

Issue:
deprecation warning incorrectly displayed
a warning is displayed even though I'm not using any templated SQL code:

![Screenshot 2023-03-03 at 5 41 40 p m](https://user-images.githubusercontent.com/989250/222855508-d27c3dbc-6558-4dea-bc65-df9bf0a84d5f.png)


Pull request:
Fix Incident - deprecation warning incorrectly displayed
## Describe your changes

Fix the bug - deprecation warning incorrectly displayed

Originally, we have the custom variable substitution, but it will also handle the connection string like this, which we have the regex to detect if it's contains `:variable`, it will detect connection string as variable substitution case.

Connection string:
`mysql+mysqldb://user:password@127.0.0.1:3306/db`

(`:password` was treat as variable)

Fix by executing the `_var_expand` after the command is parsed by `parse.parse()`, so it would not contain connecting string anymore
## Issue ticket number and link
Closes #203 

## Checklist before requesting a review

- [x] I have performed a self-review of my code
- [x] I have added thorough [tests](https://github.com/ploomber/contributing/blob/main/CONTRIBUTING.md#testing) (when necessary).
- [x] I have added the right documentation in the [docstring](https://github.com/ploomber/contributing/blob/main/CONTRIBUTING.md#documenting-changes-and-new-features) and [changelog](https://github.com/ploomber/contributing/blob/main/CONTRIBUTING.md#changelog) (when needed)



<!-- readthedocs-preview jupysql start -->
----
:books: Documentation preview :books:: https://jupysql--213.org.readthedocs.build/en/213/

<!-- readthedocs-preview jupysql end -->

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 147613c..31fdc21 100644
--- a/CHANGELOG.md
+++ b/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))
diff --git a/src/sql/command.py b/src/sql/command.py
index 11af091..56c73e0 100644
--- a/src/sql/command.py
+++ b/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 @@ class SQLCommand:
 
         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 @@ class SQLCommand:
         """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


Test patch:
diff --git a/src/tests/integration/test_generic_db_opeations.py b/src/tests/integration/test_generic_db_opeations.py
index 6adbb89..53f37e6 100644
--- a/src/tests/integration/test_generic_db_opeations.py
+++ b/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
 
 
diff --git a/src/tests/test_command.py b/src/tests/test_command.py
index 63ee871..b91d3d8 100644
--- a/src/tests/test_command.py
+++ b/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;"