Re: [PR] fix(theme-crud): enable overwrite confirmation UI for theme imports [superset]

Wed, 08 Oct 2025 02:18:32 -0700 


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


##########
superset/themes/api.py:
##########
@@ -550,15 +550,9 @@ def import_(self) -> Response:
 
         overwrite = request.form.get("overwrite") == "true"
 
-        try:
-            ImportThemesCommand(contents, overwrite=overwrite).run()
-            return self.response(200, message="Theme imported successfully")
-        except ValidationError as err:
-            logger.exception("Import themes validation error")
-            return self.response_400(message=str(err))
-        except Exception as ex:
-            logger.exception("Unexpected error importing themes")
-            return self.response_422(message=str(ex))
+        command = ImportThemesCommand(contents, overwrite=overwrite)
+        command.run()
+        return self.response(200, message="Theme imported successfully")

Review Comment:
   ### Missing Error Handling in Theme Import <sub>![category 
Design](https://img.shields.io/badge/Design-0d9488)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The import_ method removes error handling, making it impossible to 
gracefully handle ValidationError and other exceptions that may occur during 
theme import.
   
   
   ###### Why this matters
   Without proper error handling, API users won't receive meaningful error 
messages and the application may crash or return unexpected 500 errors instead 
of proper 400/422 responses.
   
   ###### Suggested change ∙ *Feature Preview*
   ```python
   try:
       command = ImportThemesCommand(contents, overwrite=overwrite)
       command.run()
       return self.response(200, message="Theme imported successfully")
   except ValidationError as err:
       logger.exception("Import themes validation error")
       return self.response_400(message=str(err))
   except Exception as ex:
       logger.exception("Unexpected error importing themes")
       return self.response_422(message=str(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/099976d2-fe28-4c02-837a-e82ff9f78e36/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/099976d2-fe28-4c02-837a-e82ff9f78e36?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/099976d2-fe28-4c02-837a-e82ff9f78e36?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/099976d2-fe28-4c02-837a-e82ff9f78e36?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/099976d2-fe28-4c02-837a-e82ff9f78e36)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:4e2a5f37-ac8d-4e5d-83e4-4dc013387d83 -->
   
   
   [](4e2a5f37-ac8d-4e5d-83e4-4dc013387d83)



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