bito-code-review[bot] commented on code in PR #40919:
URL: https://github.com/apache/superset/pull/40919#discussion_r3387690464
##########
superset/constants.py:
##########
@@ -15,17 +15,20 @@
# specific language governing permissions and limitations
# under the License.
-# ATTENTION: If you change any constants, make sure to also change
utils/common.js
+# ATTENTION: If you change any constants, make sure to also change
+# plugins/plugin-chart-echarts/src/constants.ts
# string to use when None values *need* to be converted to/from strings
from enum import Enum
from superset.utils.backports import StrEnum
+from flask_babel import gettext as __
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Use lazy_gettext for module constants</b></div>
<div id="fix">
Using `gettext` for module-level constants can fail when the module is
imported before a request context is established. The codebase consistently
uses `lazy_gettext` for constants (see `initialization/__init__.py:33` comment:
'using lazy_gettext since initialization happens prior to the request scope').
This pattern is verified across 20+ files importing `lazy_gettext` for similar
use cases.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset/constants.py
+++ b/superset/constants.py
@@ -23,7 +23,7 @@
from superset.utils.backports import StrEnum
-from flask_babel import gettext as __
+from flask_babel import lazy_gettext as __
DEFAULT_USER_AGENT = "Apache Superset"
```
</div>
</details>
</div>
<small><i>Code Review Run #91c55e</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
##########
superset-frontend/plugins/plugin-chart-echarts/src/constants.ts:
##########
@@ -27,8 +27,12 @@ import {
TitleFormData,
} from './types';
-// eslint-disable-next-line import/prefer-default-export
-export const NULL_STRING = '<NULL>';
+// ATTENTION: If you change any constants, make sure to also change
constants.py
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Documentation Inconsistency</b></div>
<div id="fix">
The comment at line 30 states 'If you change any constants, make sure to
also change constants.py', but the new TRUE_STRING and FALSE_STRING constants
lack Python counterparts. Either update the comment to scope it to
NULL_STRING/EMPTY_STRING, or add the new constants to Python to honor the
stated contract.
</div>
</div>
<small><i>Code Review Run #91c55e</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]
