codeant-ai-for-open-source[bot] commented on code in PR #38412:
URL: https://github.com/apache/superset/pull/38412#discussion_r2885767758
##########
superset/core/api/core_api_injection.py:
##########
@@ -137,24 +139,66 @@ def inject_task_implementations() -> None:
def inject_rest_api_implementations() -> None:
"""
- Replace abstract REST API functions in superset_core.api.rest_api with
concrete
- implementations from Superset.
+ Replace abstract REST API functions and decorators in
superset_core.api.rest_api
+ with concrete implementations from Superset.
"""
import superset_core.api.rest_api as core_rest_api_module
from superset.extensions import appbuilder
- def add_api(api: "type[RestApi]") -> None:
- view = appbuilder.add_api(api)
- appbuilder._add_permission(view, True)
+ T = TypeVar("T", bound=type["RestApi"])
Review Comment:
**Suggestion:** The TypeVar definition uses `type["RestApi"]` as its bound,
but the built-in `type` is not subscriptable, so this line will raise a
`TypeError` the first time `inject_rest_api_implementations` is called and
prevent REST API dependency injection from completing. Use a string forward
reference for the bound instead so it is not evaluated at runtime. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ `inject_rest_api_implementations` crashes before completing.
- ❌ `superset_core.api.rest_api.api` decorator not injected.
- ⚠️ Core REST API dependency initialization sequence is broken.
```
</details>
```suggestion
T = TypeVar("T", bound="RestApi")
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Import `inject_rest_api_implementations` from
`superset/core/api/core_api_injection.py`
(function starts at line 140 in the PR hunk).
2. Call `inject_rest_api_implementations()`; execution enters the function
body at
`superset/core/api/core_api_injection.py:140`.
3. Python evaluates the line `T = TypeVar("T", bound=type["RestApi"])` at
line 149, where
`type["RestApi"]` attempts to subscript the built-in `type`.
4. Observe a `TypeError: 'type' object is not subscriptable` raised at line
149,
preventing the function from completing and leaving
`superset_core.api.rest_api.api`
uninitialized with the concrete implementation.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/core/api/core_api_injection.py
**Line:** 149:149
**Comment:**
*Type Error: The TypeVar definition uses `type["RestApi"]` as its
bound, but the built-in `type` is not subscriptable, so this line will raise a
`TypeError` the first time `inject_rest_api_implementations` is called and
prevent REST API dependency injection from completing. Use a string forward
reference for the bound instead so it is not evaluated at runtime.
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%2F38412&comment_hash=231982699d14ffeb77ddd71c9a659e6ff4adb96710ff06ebf60b1032085523fd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38412&comment_hash=231982699d14ffeb77ddd71c9a659e6ff4adb96710ff06ebf60b1032085523fd&reaction=dislike'>👎</a>
##########
superset-core/src/superset_core/api/rest_api.py:
##########
@@ -16,20 +16,31 @@
# under the License.
"""
-REST API functions for superset-core.
+REST API functions and decorators for superset-core.
-Provides dependency-injected REST API utility functions that will be replaced
by
-host implementations during initialization.
+Provides dependency-injected REST API utility functions and decorators that
will be
+replaced by host implementations during initialization.
Usage:
- from superset_core.api.rest_api import add_api, add_extension_api
-
- add_api(MyCustomAPI)
- add_extension_api(MyExtensionAPI)
+ from superset_core.api.rest_api import api
+
+ # Unified decorator for both host and extension APIs
+ @api(
+ id="main_api",
+ name="Main API",
+ description="Primary endpoints"
+ )
+ class MyAPI(RestApi):
+ pass
"""
+from typing import Callable, TypeVar
+
from flask_appbuilder.api import BaseApi
+# Type variable for decorated API classes
+T = TypeVar("T", bound=type["RestApi"])
Review Comment:
**Suggestion:** The TypeVar bound uses the built-in `type` subscripted with
a string (`type["RestApi"]`), which causes a TypeError at import time because
built-in generics require actual types, not string forward references; use
`typing.Type` with a string forward reference instead so the module can be
imported without error. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Importing superset_core.api.rest_api raises TypeError immediately.
- ❌ RestApi base class unusable due to module import failure.
- ❌ New unified api decorator cannot be imported or applied.
```
</details>
```suggestion
from typing import Callable, TypeVar, Type
from flask_appbuilder.api import BaseApi
# Type variable for decorated API classes
T = TypeVar("T", bound=Type["RestApi"])
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Install the code containing this PR and ensure `superset-core` is on
`PYTHONPATH`.
2. In a Python shell or during application startup, import the module with
`import
superset_core.api.rest_api`, which loads
`superset-core/src/superset_core/api/rest_api.py`.
3. During module execution, Python evaluates the TypeVar declaration at
`superset-core/src/superset_core/api/rest_api.py:42`: `T = TypeVar("T",
bound=type["RestApi"])`.
4. The expression `type["RestApi"]` attempts to use the built-in generic
`type` with a
string argument, causing a `TypeError` at import time (built-in generics
require actual
types, not string forward references), preventing the module from importing
and blocking
any use of `RestApi` or `api`.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-core/src/superset_core/api/rest_api.py
**Line:** 37:42
**Comment:**
*Type Error: The TypeVar bound uses the built-in `type` subscripted
with a string (`type["RestApi"]`), which causes a TypeError at import time
because built-in generics require actual types, not string forward references;
use `typing.Type` with a string forward reference instead so the module can be
imported without error.
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%2F38412&comment_hash=cd81a2edd807fe5a5761ac6c2c5302cf247a3409e3388eb4078cd06d06dfb737&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38412&comment_hash=cd81a2edd807fe5a5761ac6c2c5302cf247a3409e3388eb4078cd06d06dfb737&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]
