Re: [PR] Fix: key error in frontend on disallowed GSheets [superset]

Fri, 21 Mar 2025 07:38:02 -0700 


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


##########
superset/views/base.py:
##########
@@ -319,7 +319,7 @@ def cached_common_bootstrap_data(  # pylint: 
disable=unused-argument
 
     # verify client has google sheets installed
     available_specs = get_available_engine_specs()
-    frontend_config["HAS_GSHEETS_INSTALLED"] = 
bool(available_specs[GSheetsEngineSpec])
+    frontend_config["HAS_GSHEETS_INSTALLED"] = GSheetsEngineSpec in 
available_specs and bool(available_specs[GSheetsEngineSpec])

Review Comment:
   ### Redundant Dictionary Lookup <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Redundant dictionary lookup after membership test causing unnecessary 
computation.
   
   ###### Why this matters
   The code checks for key existence and then performs a dictionary lookup 
which is redundant since the membership test already confirms the value exists.
   
   ###### Suggested change ∙ *Feature Preview*
   Simplify to just use the membership test since that's sufficient to 
determine if GSheetsEngineSpec is available:
   ```python
   frontend_config["HAS_GSHEETS_INSTALLED"] = GSheetsEngineSpec in 
available_specs
   ```
   
   
   ###### 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/cabd1605-0a4b-449b-83d5-e8fef7467041/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cabd1605-0a4b-449b-83d5-e8fef7467041?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/cabd1605-0a4b-449b-83d5-e8fef7467041?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/cabd1605-0a4b-449b-83d5-e8fef7467041?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cabd1605-0a4b-449b-83d5-e8fef7467041)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:4c57d247-75e9-4500-9fec-6113460531e4 -->
   
   [](4c57d247-75e9-4500-9fec-6113460531e4)



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