codeant-ai-for-open-source[bot] commented on code in PR #36880:
URL: https://github.com/apache/superset/pull/36880#discussion_r3475965188
##########
superset-frontend/src/features/databases/DatabaseModal/index.tsx:
##########
@@ -1845,14 +1880,16 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
name: target.name,
value: target.value,
});
- handleClearValidationErrors();
}}
setSSHTunnelLoginMethod={(method: AuthType) =>
setDB({
type: ActionType.SetSSHTunnelLoginMethod,
payload: { login_method: method },
})
}
+ isValidating={isValidating}
+ validationErrors={validationErrors}
+ getValidation={getBlurValidation}
Review Comment:
**Suggestion:** This wires blur-validation into the SSH tunnel form for all
modal modes, including SQLAlchemy-URI mode where `validate_parameters` is not
the right validator and often lacks the dynamic-form `parameters` payload
expected by the backend command. That can surface unrelated schema/parameter
errors and block the Connect flow after blur. Restrict `getValidation` usage to
dynamic-form mode (or provide a SQLA-safe validator for SSH fields). [api
mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ SSH blur hits dynamic validator even in SQLAlchemy-URI mode.
- ⚠️ Validation errors may reference non-existent dynamic-form fields.
- ⚠️ Extra validate_parameters calls add noise and latency.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the SQLAlchemy URI flow in the database modal so that
`useSqlAlchemyForm` is true
(set via `setDatabaseModel` assigning `configuration_method:
ConfigurationMethod.SqlalchemyUri` in
`superset-frontend/src/features/databases/DatabaseModal/index.tsx:106-117`).
2. In this mode, the BASIC tab renders `<SqlAlchemyForm>` with SSH support:
see
`index.tsx:57-87`, where the children include `<SSHTunnelSwitchComponent ...
/>` and
`{useSSHTunneling && renderSSHTunnelForm()}` once the SSH toggle is enabled.
3. `renderSSHTunnelForm` (index.tsx:175-194) renders `<SSHTunnelForm ...
isValidating={isValidating} validationErrors={validationErrors}
getValidation={getBlurValidation} />`, and `SSHTunnelForm` wires that
`getValidation` into
blur events for all SSH inputs (e.g. `validationMethods={{ onBlur:
getValidation }}` on
`server_address`, `server_port`, `username`, and `password` fields in
`superset-frontend/src/features/databases/DatabaseModal/SSHTunnelForm.tsx:92-104,109-121,129-141,179-191`).
4. On blur of any SSH field in SQLAlchemy mode, `getBlurValidation`
(index.tsx:278-291)
calls `getValidation(db)`, which is implemented by `useDatabaseValidation` in
`superset-frontend/src/views/CRUD/hooks.ts:817-845` to POST the entire `db`
object to
`/api/v1/database/validate_parameters/`. That endpoint is backed by
`DatabaseValidateParametersSchema` in `superset/databases/schemas.py:13-68`
and
`ValidateDatabaseParametersCommand` in
`superset/commands/database/validate.py:42-93`,
which assume a dynamic-form-style payload with `parameters` and
engine-specific fields
(e.g. `_get_ssh_tunnel_errors` at `validate.py:201-248` expects
`parameters.port`). In
SQLAlchemy-URI mode the payload largely lacks `parameters` and may even lack
a concrete
`engine`, so these validation calls can emit errors about missing
dynamic-form parameters
(like `port`) or payload-schema problems that do not correspond to any field
in the
SQLAlchemy UI, creating confusing or spurious validation state for SSH edits
in this mode.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9f73248f20064dbf83bc4b03d2640e0f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9f73248f20064dbf83bc4b03d2640e0f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/features/databases/DatabaseModal/index.tsx
**Line:** 1890:1892
**Comment:**
*Api Mismatch: This wires blur-validation into the SSH tunnel form for
all modal modes, including SQLAlchemy-URI mode where `validate_parameters` is
not the right validator and often lacks the dynamic-form `parameters` payload
expected by the backend command. That can surface unrelated schema/parameter
errors and block the Connect flow after blur. Restrict `getValidation` usage to
dynamic-form mode (or provide a SQLA-safe validator for SSH fields).
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36880&comment_hash=3814bf123ff1c70035b1ff227bb3e12dadb7d41b38ad83a036689a637f65a951&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36880&comment_hash=3814bf123ff1c70035b1ff227bb3e12dadb7d41b38ad83a036689a637f65a951&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]
