korbit-ai[bot] commented on code in PR #35871:
URL: https://github.com/apache/superset/pull/35871#discussion_r2500922491
##########
superset/commands/report/alert.py:
##########
@@ -81,12 +81,16 @@ def run(self) -> bool:
self._report_schedule.last_value_row_json = str(self._result)
return self._result not in (0, None, np.nan)
self._report_schedule.last_value = self._result
+
+ # Return False for None results to prevent false alert triggers
+ if self._result is None:
+ return False
try:
operator =
json.loads(self._report_schedule.validator_config_json)["op"]
threshold =
json.loads(self._report_schedule.validator_config_json)[
"threshold"
]
- return OPERATOR_FUNCTIONS[operator](self._result, threshold) #
type: ignore
+ return OPERATOR_FUNCTIONS[operator](self._result, threshold)
Review Comment:
### Inconsistent None vs zero handling in alert validation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The early return for None results bypasses the operator validation logic,
but the _validate_operator method can still set _result to 0.0 for certain
values (0, None, np.nan), creating inconsistent behavior between the two code
paths.
###### Why this matters
This inconsistency means that a query returning 0 will be evaluated against
the threshold (potentially triggering an alert), while a query returning None
will never trigger an alert, even though both are handled as edge cases in
_validate_operator.
###### Suggested change ∙ *Feature Preview*
Move the None check after the operator/threshold parsing but before the
operator function call to ensure consistent handling:
```python
try:
operator = json.loads(self._report_schedule.validator_config_json)["op"]
threshold = json.loads(self._report_schedule.validator_config_json)[
"threshold"
]
# Return False for None results to prevent false alert triggers
if self._result is None:
return False
return OPERATOR_FUNCTIONS[operator](self._result, threshold)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57038db3-dbd6-4d37-b6ed-d530ed607e0a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57038db3-dbd6-4d37-b6ed-d530ed607e0a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57038db3-dbd6-4d37-b6ed-d530ed607e0a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57038db3-dbd6-4d37-b6ed-d530ed607e0a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/57038db3-dbd6-4d37-b6ed-d530ed607e0a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:92c8255c-73f4-4eb9-8469-bcfd4e8f9466 -->
[](92c8255c-73f4-4eb9-8469-bcfd4e8f9466)
--
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]
