betodealmeida commented on a change in pull request #11755:
URL: 
https://github.com/apache/incubator-superset/pull/11755#discussion_r529965793



##########
File path: superset/datasets/api.py
##########
@@ -280,6 +284,11 @@ def put(self, pk: int) -> Response:
             500:
               $ref: '#/components/responses/500'
         """
+        override_column = (
+            request.args["override_column"]
+            if request.args.get("override_column")
+            else False
+        )

Review comment:
       You can write this as:
   
   ```python
   override_column = request.args.get("override_column", False)
   ```
   
   But I think this is not what we want, because when we pass 
`override_column=false` we would get the string `"false"`, which is a truthy 
value. You need to convert the string to a bool:
   
   ```python
   >>> from distutils.util import strtobool
   >>> strtobool("false")
   0
   >>> strtobool("true")
   1
   ```
   
   So we probably want this:
   
   ```python
   override_column = (
       bool(strtobool(request.args["override_column"]))
       if "override_column" in request.args else False
   )
   ```
   
   Can be also standardize and call this parameter `override_columns`, plural? 
Right now we have singular coming from the browser and plural in the backend.




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

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