korbit-ai[bot] commented on code in PR #33189:
URL: https://github.com/apache/superset/pull/33189#discussion_r2051490120
##########
superset/utils/core.py:
##########
@@ -1831,3 +1833,10 @@ def get_user_agent(database: Database, source:
QuerySource | None) -> str:
return user_agent_func(database, source)
return DEFAULT_USER_AGENT
+
+
+def activate_humanize_locale() -> str:
+ locale = session.get("locale", "en")
+ if (locale != "en"):
Review Comment:
### Non-Pythonic Comparison Syntax <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Unnecessary parentheses around the comparison condition make the code less
Pythonic.
###### Why this matters
Extra parentheses add visual noise and deviate from standard Python style,
making the code slightly harder to read at a glance.
###### Suggested change ∙ *Feature Preview*
```python
if locale != "en":
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/20e2a3dc-8232-40af-ac4e-33fcef98b109/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/20e2a3dc-8232-40af-ac4e-33fcef98b109?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/20e2a3dc-8232-40af-ac4e-33fcef98b109?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/20e2a3dc-8232-40af-ac4e-33fcef98b109?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/20e2a3dc-8232-40af-ac4e-33fcef98b109)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:98773cec-394e-4d2b-85dc-69463e827072 -->
[](98773cec-394e-4d2b-85dc-69463e827072)
##########
superset/constants.py:
##########
@@ -203,6 +203,25 @@ class RouteMethod: # pylint:
disable=too-few-public-methods
| EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS
)
+LOCALES_LANGUAGE_MAP = {
+ "en": "en_US",
+ "es": "es_ES",
+ "it": "it_IT",
+ "fr": "fr_FR",
+ "zh": "zh_CN",
+ "zh_TW": "zh_HK",
Review Comment:
### Unclear Regional Locale Mapping <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Potentially confusing mapping between Traditional Chinese (Taiwan) to Hong
Kong locale without explanation
###### Why this matters
The non-obvious mapping between different regions could lead to confusion or
bugs if developers don't understand why Taiwan's code maps to Hong Kong's
locale.
###### Suggested change ∙ *Feature Preview*
Either add an explanatory comment or correct the mapping if it's incorrect:
```python
"zh_TW": "zh_TW", # Traditional Chinese (Taiwan)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a946635d-276d-4004-9372-061d192445df/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a946635d-276d-4004-9372-061d192445df?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a946635d-276d-4004-9372-061d192445df?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a946635d-276d-4004-9372-061d192445df?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a946635d-276d-4004-9372-061d192445df)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:9d766310-f942-423d-abaa-7ad8dde81c7b -->
[](9d766310-f942-423d-abaa-7ad8dde81c7b)
##########
superset/models/sql_lab.py:
##########
@@ -463,10 +464,12 @@ def url(self) -> str:
@property
def last_run_humanized(self) -> str:
+ activate_humanize_locale()
return naturaltime(datetime.now() - self.changed_on)
@property
def _last_run_delta_humanized(self) -> str:
+ activate_humanize_locale()
return naturaltime(datetime.now() - self.changed_on)
Review Comment:
### Duplicate time formatting methods <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code contains two nearly identical methods that perform the same
operation, violating the DRY principle.
###### Why this matters
Duplicate code increases maintenance burden and risk of inconsistencies when
changes are needed. Any bug fixes or changes would need to be made in multiple
places.
###### Suggested change ∙ *Feature Preview*
Merge the duplicate methods into a single private method that both the
property and render method can use:
```python
def _get_humanized_time(self) -> str:
activate_humanize_locale()
return naturaltime(datetime.now() - self.changed_on)
@property
def last_run_humanized(self) -> str:
return self._get_humanized_time()
@renders("changed_on")
def last_run_delta_humanized(self) -> str:
return self._get_humanized_time()
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16333f08-1741-4ec4-aa04-e0325485eca3/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16333f08-1741-4ec4-aa04-e0325485eca3?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16333f08-1741-4ec4-aa04-e0325485eca3?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16333f08-1741-4ec4-aa04-e0325485eca3?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/16333f08-1741-4ec4-aa04-e0325485eca3)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:384eabfe-5755-4ad1-b5ac-5762071fc6e3 -->
[](384eabfe-5755-4ad1-b5ac-5762071fc6e3)
##########
superset/utils/core.py:
##########
@@ -1831,3 +1833,10 @@ def get_user_agent(database: Database, source:
QuerySource | None) -> str:
return user_agent_func(database, source)
return DEFAULT_USER_AGENT
+
+
+def activate_humanize_locale() -> str:
+ locale = session.get("locale", "en")
+ if (locale != "en"):
+ locale = LOCALES_LANGUAGE_MAP.get(locale, "fa_IR")
Review Comment:
### Inappropriate Default Fallback Locale <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The fallback locale is hardcoded to 'fa_IR' which may not be appropriate for
all non-English languages.
###### Why this matters
Using a specific language (Farsi) as a fallback for all unknown locales can
result in unexpected and incorrect translations for users of other languages.
###### Suggested change ∙ *Feature Preview*
```python
# Use 'en' as the default fallback locale
locale = LOCALES_LANGUAGE_MAP.get(locale, "en")
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1a0cce0-e70d-42a8-baab-92018a0b2e0f/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1a0cce0-e70d-42a8-baab-92018a0b2e0f?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1a0cce0-e70d-42a8-baab-92018a0b2e0f?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1a0cce0-e70d-42a8-baab-92018a0b2e0f?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1a0cce0-e70d-42a8-baab-92018a0b2e0f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:e768ac78-459b-4e2c-bfd2-cab624ac3a2f -->
[](e768ac78-459b-4e2c-bfd2-cab624ac3a2f)
--
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]
