codeant-ai-for-open-source[bot] commented on code in PR #38167:
URL: https://github.com/apache/superset/pull/38167#discussion_r2838319639
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -403,14 +416,123 @@ def backend_watcher() -> None:
click.secho("❌ No directories to watch. Exiting.", fg="red")
+def prompt_for_extension_name(
+ display_name_opt: str | None, id_opt: str | None
+) -> ExtensionNames:
+ """
+ Prompt for extension name with graceful validation and re-prompting.
+
+ Args:
+ display_name_opt: Display name provided via CLI option (if any)
+ id_opt: Extension ID provided via CLI option (if any)
+
+ Returns:
+ ExtensionNames: Validated extension name variants
+ """
+
+ # Case 1: Both provided via CLI - validate they work together
+ if display_name_opt and id_opt:
+ try:
+ # Generate all names from display name for consistency
+ temp_names = generate_extension_names(display_name_opt)
+ # Check if the provided ID matches what we'd generate
+ if temp_names["id"] == id_opt:
+ return temp_names
+ else:
+ # If IDs don't match, use the provided ID but validate it
+ validate_extension_id(id_opt)
+ validate_python_package_name(to_snake_case(id_opt))
+ validate_npm_package_name(id_opt)
+ # Create names with the provided ID (derive technical names
from ID)
+ return ExtensionNames(
+ name=display_name_opt,
+ id=id_opt,
+ mf_name=kebab_to_camel_case(id_opt),
+ backend_name=kebab_to_snake_case(id_opt),
+
backend_package=f"superset_extensions.{kebab_to_snake_case(id_opt)}",
+
backend_entry=f"superset_extensions.{kebab_to_snake_case(id_opt)}.entrypoint",
+ )
+ except ExtensionNameError as e:
+ click.secho(f"❌ {e}", fg="red")
+ sys.exit(1)
+
+ # Case 2: Only display name provided - suggest ID
+ if display_name_opt and not id_opt:
+ display_name = display_name_opt
+ try:
+ suggested_names = generate_extension_names(display_name)
+ suggested_id = suggested_names["id"]
+ except ExtensionNameError:
+ suggested_id = to_kebab_case(display_name)
+
+ extension_id = click.prompt("Extension ID", default=suggested_id,
type=str)
+
+ # Case 3: Only ID provided - ask for display name
+ elif id_opt and not display_name_opt:
+ extension_id = id_opt
+ # Validate the provided ID first
+ try:
+ validate_extension_id(id_opt)
+ except ExtensionNameError as e:
+ click.secho(f"❌ {e}", fg="red")
+ sys.exit(1)
+
+ # Suggest display name from kebab ID
+ suggested_display = " ".join(word.capitalize() for word in
id_opt.split("-"))
+ display_name = click.prompt(
+ "Extension name", default=suggested_display, type=str
+ )
+
+ # Case 4: Neither provided - ask for both
+ else:
+ display_name = click.prompt("Extension name (e.g. Hello World)",
type=str)
+ try:
+ suggested_names = generate_extension_names(display_name)
+ suggested_id = suggested_names["id"]
+ except ExtensionNameError:
+ suggested_id = to_kebab_case(display_name)
+
+ extension_id = click.prompt("Extension ID", default=suggested_id,
type=str)
+
+ # Final validation loop - try to use generate_extension_names for
consistent results
+ while True:
+ try:
+ # First try to generate from display name if possible
+ if display_name:
+ temp_names = generate_extension_names(display_name)
+ if temp_names["id"] == extension_id:
+ # Perfect match - use generated names
+ return temp_names
+
+ # If no match, validate manually and construct
+ validate_extension_id(extension_id)
+ validate_python_package_name(to_snake_case(extension_id))
+ validate_npm_package_name(extension_id)
+
+ return ExtensionNames(
+ name=display_name,
+ id=extension_id,
+ mf_name=kebab_to_camel_case(extension_id),
+ backend_name=kebab_to_snake_case(extension_id),
+
backend_package=f"superset_extensions.{kebab_to_snake_case(extension_id)}",
+
backend_entry=f"superset_extensions.{kebab_to_snake_case(extension_id)}.entrypoint",
+ )
+
+ except ExtensionNameError as e:
+ click.secho(f"❌ {e}", fg="red")
+ extension_id = click.prompt("Extension ID", type=str)
Review Comment:
**Suggestion:** In the final validation loop of the prompt function, if the
display name is invalid and causes `generate_extension_names` to raise
`ExtensionNameError`, the code keeps retrying with the same invalid display
name while only re-prompting the extension ID, leading to an unfixable loop
where the user can never progress without restarting the command. Adding a
guard so that once an error occurs, the code stops re-calling
`generate_extension_names` and falls back to validating only the extension ID
fixes this logic and avoids an effective infinite loop. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ `superset-extensions init` can hang on invalid names.
- ⚠️ User cannot recover without restarting CLI command.
```
</details>
```suggestion
can_generate_from_display = True
while True:
try:
# First try to generate from display name if possible
if display_name and can_generate_from_display:
temp_names = generate_extension_names(display_name)
if temp_names["id"] == extension_id:
# Perfect match - use generated names
return temp_names
# If no match, validate manually and construct
validate_extension_id(extension_id)
validate_python_package_name(to_snake_case(extension_id))
validate_npm_package_name(extension_id)
return ExtensionNames(
name=display_name,
id=extension_id,
mf_name=kebab_to_camel_case(extension_id),
backend_name=kebab_to_snake_case(extension_id),
backend_package=f"superset_extensions.{kebab_to_snake_case(extension_id)}",
backend_entry=f"superset_extensions.{kebab_to_snake_case(extension_id)}.entrypoint",
)
except ExtensionNameError as e:
click.secho(f"❌ {e}", fg="red")
extension_id = click.prompt("Extension ID", type=str)
can_generate_from_display = False
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the CLI command `superset-extensions init --name "<invalid name>"`
which invokes
the `init` command in
`superset-extensions-cli/src/superset_extensions_cli/cli.py:init(...)`
(around lines
214–250) with `name_opt` set and `id_opt` as `None`.
2. Inside `init`, `prompt_for_extension_name(name_opt, id_opt)` is called
(same file,
around line 200). Because only `name_opt` is provided, execution follows
"Case 2: Only
display name provided", setting `display_name = display_name_opt` and
computing/suggesting
an ID, then eventually reaching the final validation loop shown at lines
498–523.
3. In the final validation loop, `generate_extension_names(display_name)` is
called on
each iteration when `display_name` is truthy (`cli.py` lines 502–505). For
any display
name that causes `generate_extension_names()` (from
`superset_extensions_cli/utils.py`) to
raise `ExtensionNameError`—a scenario explicitly anticipated by earlier
`try/except
ExtensionNameError` blocks in this same function—the exception is caught by
the `except
ExtensionNameError` block at lines 518–523.
4. The `except` block only prompts the user for a new `extension_id`
(`extension_id =
click.prompt("Extension ID", type=str)`) and never updates `display_name`.
Control then
returns to the start of the `while True` loop where
`generate_extension_names(display_name)` is called again with the same
invalid
`display_name`, raising `ExtensionNameError` again. This creates an
effective infinite
loop where the user can change the ID indefinitely but has no way to correct
the invalid
`display_name` without aborting the command.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-extensions-cli/src/superset_extensions_cli/cli.py
**Line:** 499:523
**Comment:**
*Logic Error: In the final validation loop of the prompt function, if
the display name is invalid and causes `generate_extension_names` to raise
`ExtensionNameError`, the code keeps retrying with the same invalid display
name while only re-prompting the extension ID, leading to an unfixable loop
where the user can never progress without restarting the command. Adding a
guard so that once an error occurs, the code stops re-calling
`generate_extension_names` and falls back to validating only the extension ID
fixes this logic and avoids an effective infinite loop.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38167&comment_hash=0cf893f2909f08f33b4ea4133d8bc53e1827dced4093f686a875b0bfd22e29e5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38167&comment_hash=0cf893f2909f08f33b4ea4133d8bc53e1827dced4093f686a875b0bfd22e29e5&reaction=dislike'>👎</a>
--
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]
