You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/03/20 14:28:54 UTC

[GitHub] [superset] michael-hoffman-26 commented on a change in pull request #18968: feat(importDashboard): import api, add config_overwrite field.

michael-hoffman-26 commented on a change in pull request #18968:
URL: https://github.com/apache/superset/pull/18968#discussion_r830624771



##########
File path: superset/commands/importers/v1/__init__.py
##########
@@ -135,6 +138,8 @@ def _prevent_overwrite_existing_model(  # pylint: disable=invalid-name
         self, exceptions: List[ValidationError]
     ) -> None:
         """check if the object exists and shouldn't be overwritten"""
+        if self.config_overwrite:
+            return
         if not self.overwrite:

Review comment:
       Hi @betodealmeida,  thank you for going over this PR.  
   This PR aims to make the `"import modules flow"` more friendly, and configurability.  
   While developing this feat I got into complexibility because I didn't want to break the flow others created.  
   
   In the current `"import module flow"`, its works like this.  
   1. upload the file.  
   2. verify you don't overwrite existing models.  
   3. if so, ask for permission, then overwrite the model.      
   
   And about what you mentioned in the parenthesis ("it would fail later in the..") I didn't see it happening when I examen it.   
   I comment on the validation part (on master) and upload a dashboards model (which contains dashboards that already exist) and I didn't get any errors.  indeed the import didn't overwrite any dashboard because the default is `overwrite==false`.   
   
   In the  **new** `"import modules flow"` it will work like this.
   1. Upload your file.
   2. setup your configuration. and upload it.  
   
   According to what's going now on the code,  
   I saved backup capability to don't break the flow of validation.  
   
   But if you will agree with me, I think that now, using the new flow we can delete the validate overwriting strategy, from both backend and frontend.
   
   




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