codeant-ai-for-open-source[bot] commented on code in PR #34880:
URL: https://github.com/apache/superset/pull/34880#discussion_r3441391851
##########
superset/dashboards/api.py:
##########
@@ -1959,6 +1963,9 @@ def import_(self) -> Response:
else None
)
overwrite = request.form.get("overwrite") == "true"
+ overwrite_all = parse_boolean_string(
+ request.form.get("overwrite_all", "true" if overwrite else "false")
+ )
Review Comment:
Yes — there’s still one thing to adjust.
### What’s happening
In `superset/dashboards/api.py`, this line:
```python
overwrite_all = parse_boolean_string(
request.form.get("overwrite_all", "true" if overwrite else "false")
)
```
means:
- `overwrite=false` → `overwrite_all=false`
- `overwrite=true` and `overwrite_all` omitted → `overwrite_all=true`
So legacy/API clients that only send `overwrite=true` will now unexpectedly
overwrite related assets.
### Recommended fix
Default `overwrite_all` to `false` when it’s not explicitly provided:
```python
overwrite_all = parse_boolean_string(
request.form.get("overwrite_all", "false")
)
```
If you want to keep the UI behavior of “overwrite dashboard + related
assets” when the checkbox is used, that should still work because the frontend
explicitly sends `overwrite_all`.
### What else to check
After that, please also verify:
- the API docs/examples reflect the default clearly
- the tests cover the “`overwrite=true` but `overwrite_all` omitted” case
- any CLI behavior is still intentional and documented
So the issue is valid, and the minimal fix is to change the fallback to
`false`.
--
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]
