Re: [PR] fix(charts): add query_context validation to prevent incomplete metadata [superset]

Wed, 22 Oct 2025 07:33:51 -0700 


korbit-ai[bot] commented on code in PR #35798:
URL: https://github.com/apache/superset/pull/35798#discussion_r2452320833


##########
superset/utils/schema.py:
##########
@@ -51,3 +51,41 @@ def validate_json(value: Union[bytes, bytearray, str]) -> 
None:
         json.validate_json(value)
     except json.JSONDecodeError as ex:
         raise ValidationError("JSON not valid") from ex
+
+
+def validate_query_context_metadata(value: Union[bytes, bytearray, str, None]) 
-> None:
+    """
+    Validator for query_context field to ensure it contains required metadata.
+
+    Validates that the query_context JSON contains the required 'datasource' 
and
+    'queries' fields needed for chart data retrieval.
+
+    :raises ValidationError: if value is not valid JSON or missing required 
fields
+    :param value: a JSON string that should contain datasource and queries 
metadata
+    """
+    if value is None:
+        return  # Allow None values
+
+    try:
+        json.validate_json(value)
+        parsed_data = json.loads(value)

Review Comment:
   ### Redundant JSON validation and parsing <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The code calls json.validate_json() followed by json.loads() on the same 
value, which is redundant and inefficient since json.loads() will also validate 
the JSON.
   
   
   ###### Why this matters
   This creates unnecessary overhead by parsing and validating the JSON twice, 
and json.validate_json() may not be a standard function that exists in the json 
module.
   
   ###### Suggested change ∙ *Feature Preview*
   Remove the redundant json.validate_json() call and handle JSON parsing 
directly:
   
   ```python
   try:
       parsed_data = json.loads(value)
   except json.JSONDecodeError as ex:
       raise ValidationError("JSON not valid") from ex
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07906f0f-4dea-462c-82df-48768a89b029/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07906f0f-4dea-462c-82df-48768a89b029?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07906f0f-4dea-462c-82df-48768a89b029?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07906f0f-4dea-462c-82df-48768a89b029?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/07906f0f-4dea-462c-82df-48768a89b029)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:b129c44e-85d9-40df-8198-b9b822651c38 -->
   
   
   [](b129c44e-85d9-40df-8198-b9b822651c38)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to