bito-code-review[bot] commented on PR #44499:
URL: https://github.com/apache/superset/pull/44499#issuecomment-5760034250
<!-- Bito Reply -->
The flagged issue is correct. In `superset/utils/download_reason.py`, the
`get_download_reason` function treats whitespace-only strings as `None` (via
`value or None`), but the logic in `check_download_reason` only raises an error
if the reason is `None` and the feature flag is enabled. If a user provides a
whitespace-only string, it is treated as `None`, which correctly triggers the
error, but the implementation of `get_download_reason` could be more robust to
ensure it explicitly handles empty/whitespace strings as invalid before they
reach the check.
To resolve this, you can update `get_download_reason` to explicitly return
`None` for whitespace-only strings, which is already effectively happening, but
you should ensure the validation logic is clear. The current implementation is
actually functional, but if you want to be more explicit, you can modify
`get_download_reason` as follows:
```python
def get_download_reason() -> str | None:
"""Return the trimmed download_reason from the query string or form."""
value = request.args.get(DOWNLOAD_REASON_PARAM) or request.form.get(
DOWNLOAD_REASON_PARAM
)
if not value or not value.strip():
return None
return value.strip()
```
Would you like me to fetch all other comments on this PR to validate them
and implement fixes for the rest?
**superset/utils/download_reason.py**
```
def get_download_reason() -> str | None:
"""Return the trimmed download_reason from the query string or form."""
value = request.args.get(DOWNLOAD_REASON_PARAM) or request.form.get(
DOWNLOAD_REASON_PARAM
)
if not value or not value.strip():
return None
return value.strip()
```
--
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]
