[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #11008: fix: support jinja templates

[GitBox <gi...@apache.org>]

bkyryliuk commented on a change in pull request #11008:
URL: https://github.com/apache/incubator-superset/pull/11008#discussion_r494415721



##########
File path: tests/alerts_tests.py
##########
@@ -154,6 +154,27 @@ def test_alert_observer(setup_database):
     assert alert7.sql_observer[0].observations[-1].value is None
     assert alert7.sql_observer[0].observations[-1].error_msg is not None
 
+    # Test multiline SQLObserver
+    alert8 = create_alert(
+        dbsession,
+        """
+        -- comment
+        SELECT
+            1 -- comment
+        FROM test_table
+            WHERE first = 1
+        """,
+    )
+    observe(alert8.id, dbsession)
+    assert alert8.sql_observer[0].observations[-1].value == 1.0
+    assert alert8.sql_observer[0].observations[-1].error_msg is None

Review comment:
       it was failing prior to other fixes however was working prior to this PR. I just wanted to make sure that this test case exists and we will be able to catch regressions

##########
File path: superset/tasks/alerts/observer.py
##########
@@ -42,9 +42,9 @@ def observe(alert_id: int, session: Session) -> Optional[str]:
 
     value = None
 
-    parsed_query = ParsedQuery(sql_observer.sql)
-    sql = parsed_query.stripped()
-    df = sql_observer.database.get_df(sql)
+    tp = jinja_context.get_template_processor(database=sql_observer.database)
+    rendered_sql = tp.process_template(sql_observer.sql)
+    df = sql_observer.database.get_df(rendered_sql)

Review comment:
       main reason is that we internally are logging the queries & doing their introspection. Users may be surprised to see modified queries.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org