codeant-ai-for-open-source[bot] commented on code in PR #40654:
URL: https://github.com/apache/superset/pull/40654#discussion_r3407882860
##########
superset/utils/encrypt.py:
##########
@@ -19,17 +19,76 @@
from dataclasses import dataclass
from typing import Any, Optional
-from flask import Flask
+from flask import current_app, Flask
from flask_babel import lazy_gettext as _
from sqlalchemy import Table, text, TypeDecorator
from sqlalchemy.engine import Connection, Dialect, Row
from sqlalchemy_utils import EncryptedType as SqlaEncryptedType
+from sqlalchemy_utils.types.encrypted.encrypted_type import (
+ AesEngine,
+ AesGcmEngine,
+ EncryptionDecryptionBaseEngine,
+)
class EncryptedType(SqlaEncryptedType):
cache_ok = True
+# Named encryption engines selectable via the
``SQLALCHEMY_ENCRYPTED_FIELD_ENGINE``
+# config. "aes" (AES-CBC) is the historical default; "aes-gcm" is authenticated
+# encryption (recommended for new deployments). NOTE: switching an existing
+# deployment from "aes" to "aes-gcm" requires re-encrypting all stored secrets
+# first — see the SIP referenced in the docs. Changing this on a populated
+# database without that migration will make existing secrets undecryptable.
+ENCRYPTION_ENGINES: dict[str, type[EncryptionDecryptionBaseEngine]] = {
+ "aes": AesEngine,
+ "aes-gcm": AesGcmEngine,
+}
+
+# The historical fallback engine when the config does not name one.
+DEFAULT_ENCRYPTION_ENGINE_NAME = "aes"
+
+# Engines whose ciphertext is authenticated: a successful decrypt is
+# cryptographic proof the value is genuinely in that form. AES-GCM carries an
+# authentication tag; AES-CBC does not, so a CBC "success" can be coincidental.
+# Classification logic (the migrator's idempotency fast path) must let an
+# authenticated decrypt win over an unauthenticated one, never the reverse.
+AUTHENTICATED_ENGINES: frozenset[type[EncryptionDecryptionBaseEngine]] =
frozenset(
+ {AesGcmEngine}
+)
+
+
+def _is_authenticated_engine(engine: type[EncryptionDecryptionBaseEngine]) ->
bool:
+ return engine in AUTHENTICATED_ENGINES
Review Comment:
**Suggestion:** Add an inline docstring to this newly introduced helper
function so it complies with the rule that new functions must be documented.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python function and it has no docstring. The custom
rule requires new functions to be documented inline, so this is a real
violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=581c3b610b0d4c838e6583712e1656b0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=581c3b610b0d4c838e6583712e1656b0&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/utils/encrypt.py
**Line:** 62:63
**Comment:**
*Custom Rule: Add an inline docstring to this newly introduced helper
function so it complies with the rule that new functions must be documented.
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%2F40654&comment_hash=c942b4a718364edbc21962d5dd4e5c7d95254e40c71215275da431d6b0308233&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40654&comment_hash=c942b4a718364edbc21962d5dd4e5c7d95254e40c71215275da431d6b0308233&reaction=dislike'>👎</a>
##########
superset/utils/encrypt.py:
##########
@@ -101,10 +179,40 @@ def created_by_enc_field_factory(field: TypeDecorator) ->
bool:
class SecretsMigrator:
- def __init__(self, previous_secret_key: str) -> None:
+ """Re-encrypts every app-encrypted column in the ORM.
+
+ Two modes, which can also be combined:
+
+ - **Key rotation** — pass ``previous_secret_key``. Values are decrypted
+ under the previous key and re-encrypted under the current ``SECRET_KEY``
+ using the currently-configured engine.
+ - **Engine migration** — pass ``target_engine`` (e.g. ``AesGcmEngine``).
+ Values are decrypted under the current ``SECRET_KEY`` with the source
+ engine and re-encrypted with the target engine. This is how an existing
+ install moves from AES-CBC to authenticated AES-GCM without bricking
+ stored secrets; the ``SECRET_KEY`` itself is unchanged.
+
+ Both modes share the same all-or-nothing transaction and per-column
+ idempotency: a value already readable in the target form is left untouched,
+ so a run can be safely repeated or resumed.
+ """
+
+ def __init__(
+ self,
+ previous_secret_key: Optional[str] = None,
+ target_engine: Optional[type[Any]] = None,
+ ) -> None:
from superset import db # pylint: disable=import-outside-toplevel
Review Comment:
**Suggestion:** Add a docstring to this newly added initializer method to
explain its migration-mode behavior and parameter intent. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added method has no docstring. The rule requires new Python
functions and classes to include docstrings, so the suggestion identifies a
genuine violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=97c5d63cebde42d5a54df73a874d8264&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=97c5d63cebde42d5a54df73a874d8264&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/utils/encrypt.py
**Line:** 200:205
**Comment:**
*Custom Rule: Add a docstring to this newly added initializer method to
explain its migration-mode behavior and parameter intent.
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%2F40654&comment_hash=e53f6c6f58c79592f187e49ff4d6175477578f6d28361ec833b2d9ced254dd76&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40654&comment_hash=e53f6c6f58c79592f187e49ff4d6175477578f6d28361ec833b2d9ced254dd76&reaction=dislike'>👎</a>
##########
superset/cli/update.py:
##########
@@ -110,17 +110,37 @@ def update_api_docs() -> None:
help="An optional previous secret key, if PREVIOUS_SECRET_KEY "
"is not set on the config",
)
-def re_encrypt_secrets(previous_secret_key: Optional[str] = None) -> None:
[email protected](
+ "--engine",
+ "-e",
+ "target_engine_name",
+ required=False,
+ type=click.Choice(sorted(ENCRYPTION_ENGINES), case_sensitive=False),
+ help="Re-encrypt all app-encrypted fields with this encryption engine "
+ "(e.g. 'aes-gcm' for authenticated encryption). The SECRET_KEY is "
+ "unchanged. Take a metadata-DB backup first, then set "
+ "SQLALCHEMY_ENCRYPTED_FIELD_ENGINE to the same value and restart.",
+)
+def re_encrypt_secrets(
+ previous_secret_key: Optional[str] = None,
+ target_engine_name: Optional[str] = None,
+) -> None:
Review Comment:
**Suggestion:** Add a concise docstring immediately under this new function
definition to document its behavior, inputs, and expected CLI usage.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The custom rule requires newly added Python functions to include docstrings.
This is a newly introduced function definition and the final file state shows
no docstring immediately under it, so the suggestion identifies a real
violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=27e770b7cc2248538fcf75f16a891e4a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=27e770b7cc2248538fcf75f16a891e4a&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/cli/update.py
**Line:** 124:127
**Comment:**
*Custom Rule: Add a concise docstring immediately under this new
function definition to document its behavior, inputs, and expected CLI usage.
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%2F40654&comment_hash=f0af1c9170fa43c8aa4855f5271ef2dc4ac0d36f08f6bba46a398a77ee7f2d9e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40654&comment_hash=f0af1c9170fa43c8aa4855f5271ef2dc4ac0d36f08f6bba46a398a77ee7f2d9e&reaction=dislike'>👎</a>
##########
tests/integration_tests/cli_tests.py:
##########
@@ -364,3 +366,94 @@ def
test_re_encrypt_secrets_failure_exits_nonzero(app_context):
# The failure path must be handled by the CLI, not leaked as an
# uncaught exception.
assert response.exception is None or isinstance(response.exception,
SystemExit)
+
+
+def test_re_encrypt_secrets_engine_option_invokes_migrator(app_context) ->
None:
Review Comment:
**Suggestion:** Add a type annotation for the `app_context` fixture
parameter so the new test function has fully typed inputs. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python test function and its `app_context` parameter
is unannotated. The custom rule requires new or modified Python functions to be
fully typed when surrounding code is changing, so this is a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=23ae0a8654994a389f6e86e8f2a87c23&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=23ae0a8654994a389f6e86e8f2a87c23&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:** tests/integration_tests/cli_tests.py
**Line:** 371:371
**Comment:**
*Custom Rule: Add a type annotation for the `app_context` fixture
parameter so the new test function has fully typed inputs.
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%2F40654&comment_hash=b1fc85412ff923022ef4241d5f37f17095c7ca125da3ce7553704f0dd1bf2b19&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40654&comment_hash=b1fc85412ff923022ef4241d5f37f17095c7ca125da3ce7553704f0dd1bf2b19&reaction=dislike'>👎</a>
##########
tests/integration_tests/cli_tests.py:
##########
@@ -364,3 +366,94 @@ def
test_re_encrypt_secrets_failure_exits_nonzero(app_context):
# The failure path must be handled by the CLI, not leaked as an
# uncaught exception.
assert response.exception is None or isinstance(response.exception,
SystemExit)
+
+
+def test_re_encrypt_secrets_engine_option_invokes_migrator(app_context) ->
None:
+ """
+ When --engine is provided, the CLI must resolve the engine name to the
+ correct engine class and pass it to SecretsMigrator as target_engine.
+ """
+ current_app.config.pop("PREVIOUS_SECRET_KEY", None)
+ runner = current_app.test_cli_runner()
+ with mock.patch.object(
+ superset.cli.update,
+ "SecretsMigrator",
+ ) as migrator_mock:
+ migrator_mock.return_value.run.return_value = (
+ superset.utils.encrypt.ReEncryptStats()
+ )
+ response = runner.invoke(
+ superset.cli.update.re_encrypt_secrets,
+ ["--engine", "aes-gcm"],
+ )
+
+ assert response.exit_code == 0
+ call_kwargs = migrator_mock.call_args.kwargs
+ assert call_kwargs.get("target_engine") is AesGcmEngine
+ assert call_kwargs.get("previous_secret_key") is None
+
+
+def test_re_encrypt_secrets_engine_option_case_insensitive(app_context) ->
None:
Review Comment:
**Suggestion:** Add a type annotation for the `app_context` fixture
parameter to comply with the full typing requirement for newly added functions.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly introduced function and the `app_context` fixture parameter
lacks a type hint. That violates the rule requiring new Python functions to be
fully typed.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=62d1ceb53f204ead83b2b18f00081995&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=62d1ceb53f204ead83b2b18f00081995&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:** tests/integration_tests/cli_tests.py
**Line:** 396:396
**Comment:**
*Custom Rule: Add a type annotation for the `app_context` fixture
parameter to comply with the full typing requirement for newly added functions.
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%2F40654&comment_hash=efdd98561817c255b3efbb77941f84ac847838eecb91eaefa9bb4a5f98b2c439&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40654&comment_hash=efdd98561817c255b3efbb77941f84ac847838eecb91eaefa9bb4a5f98b2c439&reaction=dislike'>👎</a>
##########
tests/integration_tests/cli_tests.py:
##########
@@ -364,3 +366,94 @@ def
test_re_encrypt_secrets_failure_exits_nonzero(app_context):
# The failure path must be handled by the CLI, not leaked as an
# uncaught exception.
assert response.exception is None or isinstance(response.exception,
SystemExit)
+
+
+def test_re_encrypt_secrets_engine_option_invokes_migrator(app_context) ->
None:
+ """
+ When --engine is provided, the CLI must resolve the engine name to the
+ correct engine class and pass it to SecretsMigrator as target_engine.
+ """
+ current_app.config.pop("PREVIOUS_SECRET_KEY", None)
+ runner = current_app.test_cli_runner()
+ with mock.patch.object(
+ superset.cli.update,
+ "SecretsMigrator",
+ ) as migrator_mock:
+ migrator_mock.return_value.run.return_value = (
+ superset.utils.encrypt.ReEncryptStats()
+ )
+ response = runner.invoke(
+ superset.cli.update.re_encrypt_secrets,
+ ["--engine", "aes-gcm"],
+ )
+
+ assert response.exit_code == 0
+ call_kwargs = migrator_mock.call_args.kwargs
+ assert call_kwargs.get("target_engine") is AesGcmEngine
+ assert call_kwargs.get("previous_secret_key") is None
+
+
+def test_re_encrypt_secrets_engine_option_case_insensitive(app_context) ->
None:
+ """
+ The --engine option must be case-insensitive per
+ click.Choice(..., case_sensitive=False).
+ """
+ current_app.config.pop("PREVIOUS_SECRET_KEY", None)
+ runner = current_app.test_cli_runner()
+ with mock.patch.object(
+ superset.cli.update,
+ "SecretsMigrator",
+ ) as migrator_mock:
+ migrator_mock.return_value.run.return_value = (
+ superset.utils.encrypt.ReEncryptStats()
+ )
+ response = runner.invoke(
+ superset.cli.update.re_encrypt_secrets,
+ ["--engine", "AES-GCM"],
+ )
+
+ assert response.exit_code == 0
+ assert migrator_mock.call_args.kwargs.get("target_engine") is AesGcmEngine
+
+
+def test_re_encrypt_secrets_combined_key_rotation_and_engine(app_context) ->
None:
+ """
+ --previous_secret_key and --engine combine in a single run: the migrator
+ must receive both the previous key (for decryption) and the target engine
+ (for re-encryption). This is the mode most likely to regress, since the
+ single-option tests each pin only the other's variable.
+ """
+ current_app.config.pop("PREVIOUS_SECRET_KEY", None)
+ runner = current_app.test_cli_runner()
+ with mock.patch.object(
+ superset.cli.update,
+ "SecretsMigrator",
+ ) as migrator_mock:
+ migrator_mock.return_value.run.return_value = (
+ superset.utils.encrypt.ReEncryptStats()
+ )
+ response = runner.invoke(
+ superset.cli.update.re_encrypt_secrets,
+ ["--previous_secret_key", "old-key", "--engine", "aes-gcm"],
+ )
+
+ assert response.exit_code == 0
+ call_kwargs = migrator_mock.call_args.kwargs
+ assert call_kwargs.get("target_engine") is AesGcmEngine
+ assert call_kwargs.get("previous_secret_key") == "old-key"
+
+
+def test_re_encrypt_secrets_engine_option_invalid_raises_usage(app_context) ->
None:
Review Comment:
**Suggestion:** Annotate the `app_context` fixture argument with an
appropriate type to satisfy the rule requiring typed parameters in new
functions. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is also a newly added Python function, and its parameter is missing a
type annotation. That matches the custom rule about fully typing new Python
code.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d69e2cc225e244d48dc74709586dacfd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d69e2cc225e244d48dc74709586dacfd&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:** tests/integration_tests/cli_tests.py
**Line:** 446:446
**Comment:**
*Custom Rule: Annotate the `app_context` fixture argument with an
appropriate type to satisfy the rule requiring typed parameters in new
functions.
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%2F40654&comment_hash=b0edc54bff499876180975d11cdd37815615030544ef760f7d4ff594a8ea4350&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40654&comment_hash=b0edc54bff499876180975d11cdd37815615030544ef760f7d4ff594a8ea4350&reaction=dislike'>👎</a>
##########
tests/integration_tests/cli_tests.py:
##########
@@ -364,3 +366,94 @@ def
test_re_encrypt_secrets_failure_exits_nonzero(app_context):
# The failure path must be handled by the CLI, not leaked as an
# uncaught exception.
assert response.exception is None or isinstance(response.exception,
SystemExit)
+
+
+def test_re_encrypt_secrets_engine_option_invokes_migrator(app_context) ->
None:
+ """
+ When --engine is provided, the CLI must resolve the engine name to the
+ correct engine class and pass it to SecretsMigrator as target_engine.
+ """
+ current_app.config.pop("PREVIOUS_SECRET_KEY", None)
+ runner = current_app.test_cli_runner()
+ with mock.patch.object(
+ superset.cli.update,
+ "SecretsMigrator",
+ ) as migrator_mock:
+ migrator_mock.return_value.run.return_value = (
+ superset.utils.encrypt.ReEncryptStats()
+ )
+ response = runner.invoke(
+ superset.cli.update.re_encrypt_secrets,
+ ["--engine", "aes-gcm"],
+ )
+
+ assert response.exit_code == 0
+ call_kwargs = migrator_mock.call_args.kwargs
+ assert call_kwargs.get("target_engine") is AesGcmEngine
+ assert call_kwargs.get("previous_secret_key") is None
+
+
+def test_re_encrypt_secrets_engine_option_case_insensitive(app_context) ->
None:
+ """
+ The --engine option must be case-insensitive per
+ click.Choice(..., case_sensitive=False).
+ """
+ current_app.config.pop("PREVIOUS_SECRET_KEY", None)
+ runner = current_app.test_cli_runner()
+ with mock.patch.object(
+ superset.cli.update,
+ "SecretsMigrator",
+ ) as migrator_mock:
+ migrator_mock.return_value.run.return_value = (
+ superset.utils.encrypt.ReEncryptStats()
+ )
+ response = runner.invoke(
+ superset.cli.update.re_encrypt_secrets,
+ ["--engine", "AES-GCM"],
+ )
+
+ assert response.exit_code == 0
+ assert migrator_mock.call_args.kwargs.get("target_engine") is AesGcmEngine
+
+
+def test_re_encrypt_secrets_combined_key_rotation_and_engine(app_context) ->
None:
Review Comment:
**Suggestion:** Add an explicit type hint to the `app_context` parameter so
this newly introduced test function is fully typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The function is newly added and still has an untyped `app_context`
parameter. Since the surrounding code is part of the PR, the custom typing rule
applies and this is a genuine violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c43f6c559faa410ab97641e518365ae7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c43f6c559faa410ab97641e518365ae7&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:** tests/integration_tests/cli_tests.py
**Line:** 419:419
**Comment:**
*Custom Rule: Add an explicit type hint to the `app_context` parameter
so this newly introduced test function is fully typed.
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%2F40654&comment_hash=1aa354ee336d05865155157c13cac497ddb2a5ee937a738c27e0e781eb37c334&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40654&comment_hash=1aa354ee336d05865155157c13cac497ddb2a5ee937a738c27e0e781eb37c334&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]
