bito-code-review[bot] commented on code in PR #43350:
URL: https://github.com/apache/superset/pull/43350#discussion_r3818310586
##########
superset/commands/chart/update.py:
##########
@@ -78,6 +88,15 @@ def run(self) -> Model:
self._properties["last_saved_at"] = datetime.now()
self._properties["last_saved_by"] = g.user
+ if "params" in self._properties:
+ register_matching_normalization_context(
+ db.session,
+ self._model.id,
+ self._normalization_changes,
+ self._model.params,
+ self._properties["params"],
+ )
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing guard bypasses short-circuit</b></div>
<div id="fix">
The condition on line 91 dropped the `self._normalization_changes is not
None` check. When `normalization_changes` is `None`, the new path still parses
`self._model.params` and `self._properties["params"]` into Python dicts and
calls `register_matching_normalization_context` before the function's internal
guard at line 233 returns early. This is wasteful and inconsistent with the
original behavior where no parsing occurred at all when `normalization_changes`
was absent. Restore the guard to preserve the short-circuit.
</div>
</div>
<small><i>Code Review Run #1dca38</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
