Re: [PR] feat(database): Adding per-user caching option in Security tab [superset]

Mon, 15 Sep 2025 01:18:10 -0700 


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


##########
superset/common/query_object.py:
##########
@@ -454,13 +454,19 @@ def cache_key(self, **extra: Any) -> str:  # noqa: C901
             cache_dict["annotation_layers"] = annotation_layers
 
         # Add an impersonation key to cache if impersonation is enabled on the 
db
-        # or if the CACHE_QUERY_BY_USER flag is on
+        # or if the CACHE_QUERY_BY_USER flag is on or per_user_caching is 
enabled on
+        #  the database
         try:
             database = self.datasource.database  # type: ignore
+            extra = json.loads(database.extra or "{}")

Review Comment:
   ### Repeated JSON parsing in cache_key method <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   JSON parsing is performed on every cache_key() call without caching the 
parsed result.
   
   
   ###### Why this matters
   This causes unnecessary JSON parsing overhead on every cache key generation, 
which can be frequent in high-traffic scenarios. The database.extra field is 
unlikely to change during the lifetime of a request, making this repeated 
parsing wasteful.
   
   ###### Suggested change ∙ *Feature Preview*
   Cache the parsed JSON result or move the parsing outside the cache_key 
method. Consider parsing database.extra once when the QueryObject is created 
and storing it as a parsed attribute, or implement memoization for the parsed 
extra field on the database object.
   
   
   ###### 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/3617ebca-d5f3-44a5-9224-d59f7cd01603/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3617ebca-d5f3-44a5-9224-d59f7cd01603?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/3617ebca-d5f3-44a5-9224-d59f7cd01603?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/3617ebca-d5f3-44a5-9224-d59f7cd01603?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3617ebca-d5f3-44a5-9224-d59f7cd01603)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:d773bd79-d4d0-4001-9cdc-5ae6dc08d1da -->
   
   
   [](d773bd79-d4d0-4001-9cdc-5ae6dc08d1da)



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