[GitHub] [superset] ktmud commented on a diff in pull request #20059: feat: Save column data into json_metadata for all Query executions

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

ktmud commented on code in PR #20059:
URL: https://github.com/apache/superset/pull/20059#discussion_r874208080


##########
superset/queries/dao.py:
##########
@@ -48,3 +50,20 @@ def update_saved_query_exec_info(query_id: int) -> None:
                 saved_query.rows = query.rows
                 saved_query.last_run = datetime.now()
             db.session.commit()
+
+    @staticmethod
+    def save_metadata(query: Query, query_payload: str) -> None:
+        # parse payload
+        try:
+            payload: Dict[str, Any] = json.loads(query_payload)

Review Comment:
   The fact that we are dumping a json then reloading it within the same process doesn't bod well for me. It's probably also not a great idea to passing strings around between functions.
   
   Can we refactor `_execution_context_convertor` to return the raw dict instead? (Maybe add a new public function?)



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

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