You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "villebro (via GitHub)" <gi...@apache.org> on 2026/03/03 09:26:12 UTC
[PR] chore(extensions): clean up backend entrypoints and file globs [superset]
villebro opened a new pull request, #38360:
URL: https://github.com/apache/superset/pull/38360
### SUMMARY
Simplifies extension development by establishing **convention-over-configuration** for entry points. Eliminates the need to configure `entryPoints` arrays by requiring standard file locations instead.
**Before:**
```json
{
"backend": {
"entryPoints": ["my_extension.entrypoint"],
"files": ["backend/src/**/*.py"]
}
}
```
**After:**
- Backend entry point: `backend/src/superset_extensions/{publisher}/{name}/entrypoint.py`
- Frontend entry point: `frontend/src/index.tsx`
- Build config: `backend/pyproject.toml`
## Breaking Changes
⚠️ **Extensions must migrate** to use conventional entry points:
1. **Remove `backend` section** from `extension.json`
2. **Move entry point** to: `backend/src/superset_extensions/{publisher_snake}/{name_snake}/entrypoint.py`
3. **Add build config** to `backend/pyproject.toml`:
```toml
[tool.apache_superset_extensions.build]
include = ["src/superset_extensions/{publisher}/{name}/**/*.py"]
exclude = []
```
## Key Changes
- **Schema**: Removed `entryPoints` field from `ExtensionConfigBackend` and `ManifestBackend`
- **CLI**: Updated validation, build logic, and templates for convention-based approach
- **Templates**: Backend config moved from `extension.json` to `pyproject.toml`
- **Tests**: Updated all tests to work with new approach
- **Docs**: Updated developer documentation
## Benefits
✅ **Simpler development** - No configuration needed, predictable file locations
✅ **Python standards** - Uses `pyproject.toml` following PEP 518/621
✅ **Future-ready** - Foundation for automatic contribution discovery
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2879475573
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -217,17 +222,36 @@ def copy_frontend_dist(cwd: Path) -> str:
def copy_backend_files(cwd: Path) -> None:
+ """Copy backend files based on pyproject.toml build configuration (validation already passed)."""
dist_dir = cwd / "dist"
- extension = read_json(cwd / "extension.json")
- if not extension:
- click.secho("❌ No extension.json file found.", err=True, fg="red")
- sys.exit(1)
+ backend_dir = cwd / "backend"
- for pat in extension.get("backend", {}).get("files", []):
- for f in cwd.glob(pat):
+ # Read build config from pyproject.toml
+ pyproject = read_toml(backend_dir / "pyproject.toml")
+ assert pyproject
+ build_config = (
+ pyproject.get("tool", {}).get("apache_superset_extensions", {}).get("build", {})
+ )
+ include_patterns = build_config.get("include", [])
+ exclude_patterns = build_config.get("exclude", [])
+
+ # Process include patterns
+ for pattern in include_patterns:
Review Comment:
<!-- Bito Reply -->
This question isn’t related to the pull request. I can only help with questions about the PR’s code or comments.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2879463936
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -217,17 +222,36 @@ def copy_frontend_dist(cwd: Path) -> str:
def copy_backend_files(cwd: Path) -> None:
+ """Copy backend files based on pyproject.toml build configuration (validation already passed)."""
dist_dir = cwd / "dist"
- extension = read_json(cwd / "extension.json")
- if not extension:
- click.secho("❌ No extension.json file found.", err=True, fg="red")
- sys.exit(1)
+ backend_dir = cwd / "backend"
Review Comment:
<!-- Bito Reply -->
The suggestion replaces a bare `assert pyproject` with an explicit check that provides a clear error message and exits gracefully if `pyproject.toml` can't be read. This improves error handling in the `copy_backend_files` function, preventing unhandled AssertionErrors and ensuring consistent CLI behavior.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2877259730
##########
docs/developer_docs/extensions/development.md:
##########
@@ -100,15 +100,36 @@ The `extension.json` file contains the metadata necessary for the host applicati
"displayName": "Dataset References",
"version": "1.0.0",
"license": "Apache-2.0",
- "backend": {
- "entryPoints": ["superset_extensions.dataset_references.entrypoint"],
- "files": ["backend/src/superset_extensions/dataset_references/**/*.py"]
- },
"permissions": []
}
```
-The `backend` section specifies Python entry points to load eagerly when the extension starts, and glob patterns for source files to include in the bundle.
+### Convention-Based Entry Points
+
+Extensions use standardized entry point locations:
+
+- **Backend**: `backend/src/superset_extensions/{publisher}/{name}/entrypoint.py`
Review Comment:
This convention is already part of the framework. Without this namespacing, an extension with the publisher `pandas` would be able to override functionality in the `pandas` dependency. This just ensures that all extensions are collision free, both among external libraries, but also other extensions.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2877246188
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -217,17 +222,36 @@ def copy_frontend_dist(cwd: Path) -> str:
def copy_backend_files(cwd: Path) -> None:
+ """Copy backend files based on pyproject.toml build configuration (validation already passed)."""
dist_dir = cwd / "dist"
- extension = read_json(cwd / "extension.json")
- if not extension:
- click.secho("❌ No extension.json file found.", err=True, fg="red")
- sys.exit(1)
+ backend_dir = cwd / "backend"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Use of assert statement detected</b></div>
<div id="fix">
Line 227 uses `assert` which should be replaced with proper error handling. Use explicit validation with `if` statement and `sys.exit()` instead.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
# Read build config from pyproject.toml
pyproject = read_toml(backend_dir / "pyproject.toml")
if not pyproject:
click.secho("❌ Failed to read backend pyproject.toml", err=True, fg="red")
sys.exit(1)
build_config = (
````
</div>
</details>
</div>
<small><i>Code Review Run #2c389c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-extensions-cli/tests/test_templates.py:
##########
@@ -81,23 +81,16 @@ def test_extension_json_template_renders_with_both_frontend_and_backend(
# Verify frontend section is not present (contributions are code-first)
assert "frontend" not in parsed
- # Verify backend section exists
- assert "backend" in parsed
- backend = parsed["backend"]
- assert backend["entryPoints"] == [
- "superset_extensions.test_org.test_extension.entrypoint"
- ]
- assert backend["files"] == [
- "backend/src/superset_extensions/test_org/test_extension/**/*.py"
- ]
+ # Verify no backend section in extension.json (moved to pyproject.toml)
+ assert "backend" not in parsed
@pytest.mark.unit
@pytest.mark.parametrize(
"include_frontend,include_backend,expected_sections",
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Parametrize decorator argument type error</b></div>
<div id="fix">
Line 90: `pytest.mark.parametrize` first argument should be a `tuple` of parameter names, not a string. Change `"include_frontend,include_backend,expected_sections"` to `("include_frontend", "include_backend", "expected_sections")` or use a tuple.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
("include_frontend", "include_backend", "expected_sections"),
````
</div>
</details>
</div>
<small><i>Code Review Run #2c389c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -217,17 +222,36 @@ def copy_frontend_dist(cwd: Path) -> str:
def copy_backend_files(cwd: Path) -> None:
+ """Copy backend files based on pyproject.toml build configuration (validation already passed)."""
dist_dir = cwd / "dist"
- extension = read_json(cwd / "extension.json")
- if not extension:
- click.secho("❌ No extension.json file found.", err=True, fg="red")
- sys.exit(1)
+ backend_dir = cwd / "backend"
- for pat in extension.get("backend", {}).get("files", []):
- for f in cwd.glob(pat):
+ # Read build config from pyproject.toml
+ pyproject = read_toml(backend_dir / "pyproject.toml")
+ assert pyproject
+ build_config = (
+ pyproject.get("tool", {}).get("apache_superset_extensions", {}).get("build", {})
+ )
+ include_patterns = build_config.get("include", [])
+ exclude_patterns = build_config.get("exclude", [])
+
+ # Process include patterns
+ for pattern in include_patterns:
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Bug in file copying logic</b></div>
<div id="fix">
The current implementation splits patterns on '/' and constructs glob calls that fail for patterns without '/' or starting with '**'. Simplify to use backend_dir.glob(pattern) directly, which handles all standard glob patterns correctly.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
for pattern in include_patterns:
for f in backend_dir.glob(pattern):
if not f.is_file():
````
</div>
</details>
</div>
<small><i>Code Review Run #2c389c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -272,6 +296,89 @@ def app() -> None:
def validate() -> None:
validate_npm()
+ cwd = Path.cwd()
+
+ # Validate extension.json exists and is valid
+ extension_data = read_json(cwd / "extension.json")
+ if not extension_data:
+ click.secho("❌ extension.json not found.", err=True, fg="red")
+ sys.exit(1)
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Blind exception catch without specific type</b></div>
<div id="fix">
Line 305 catches a blind `Exception`. Catch specific exception types (e.g., `ValueError`, `ValidationError`) instead to improve error handling.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
try:
extension = ExtensionConfig.model_validate(extension_data)
except (ValueError, TypeError) as e:
click.secho(f"❌ Invalid extension.json: {e}", err=True, fg="red")
````
</div>
</details>
</div>
<small><i>Code Review Run #2c389c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2877238680
##########
docs/developer_docs/extensions/development.md:
##########
@@ -100,15 +100,36 @@ The `extension.json` file contains the metadata necessary for the host applicati
"displayName": "Dataset References",
"version": "1.0.0",
"license": "Apache-2.0",
- "backend": {
- "entryPoints": ["superset_extensions.dataset_references.entrypoint"],
- "files": ["backend/src/superset_extensions/dataset_references/**/*.py"]
- },
"permissions": []
}
```
-The `backend` section specifies Python entry points to load eagerly when the extension starts, and glob patterns for source files to include in the bundle.
+### Convention-Based Entry Points
+
+Extensions use standardized entry point locations:
+
+- **Backend**: `backend/src/superset_extensions/{publisher}/{name}/entrypoint.py`
Review Comment:
This structure would be a bit weird when working on an extension, each extension would have `backend/src/superset_extensions` directory right? Having this superset_extensions directory inside an extension seems wrong to me. Is my understanding flawed here?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2879458754
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -217,17 +222,36 @@ def copy_frontend_dist(cwd: Path) -> str:
def copy_backend_files(cwd: Path) -> None:
+ """Copy backend files based on pyproject.toml build configuration (validation already passed)."""
dist_dir = cwd / "dist"
- extension = read_json(cwd / "extension.json")
- if not extension:
- click.secho("❌ No extension.json file found.", err=True, fg="red")
- sys.exit(1)
+ backend_dir = cwd / "backend"
Review Comment:
Same as previous reply - we have a validation step that ensures all the requirements here are satisfied.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38360:
URL: https://github.com/apache/superset/pull/38360#issuecomment-3989743895
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2879437707
##########
tests/unit_tests/extensions/test_types.py:
##########
@@ -236,7 +239,9 @@ def test_manifest_backend_no_files_field():
"publisher": "my-org",
"name": "my-extension",
"displayName": "My Extension",
- "backend": {"entryPoints": ["my_extension.entrypoint"]},
+ "backend": {
+ "entrypoint": "superset_extensions.my_org.my_extension.entrypoint"
+ },
}
)
# ManifestBackend should not have a 'files' field
Review Comment:
**Suggestion:** The test that asserts `ManifestBackend` has no `files` field never verifies that a backend object is actually created, so if `Manifest` stops populating `backend`, the test will still pass because `hasattr(None, "files")` is false, hiding potential regressions. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Missing backend object would not fail this test.
- ⚠️ Weak manifest schema validation for backend configuration.
```
</details>
```suggestion
assert manifest.backend is not None
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run `pytest
tests/unit_tests/extensions/test_types.py::test_manifest_backend_no_files_field`.
2. Inspect the test at `tests/unit_tests/extensions/test_types.py:234-248`; it calls
`Manifest.model_validate` and then evaluates `hasattr(manifest.backend, "files")` without
checking `manifest.backend` is not None.
3. In Python, confirm `hasattr(None, "files")` returns `False`, matching the test's
expected assertion.
4. Therefore, if a regression causes `Manifest.model_validate` to stop populating
`backend` (leaving it as None for this input), this test would still pass, failing to
catch the schema regression around backend creation.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/extensions/test_types.py
**Line:** 247:247
**Comment:**
*Logic Error: The test that asserts `ManifestBackend` has no `files` field never verifies that a backend object is actually created, so if `Manifest` stops populating `backend`, the test will still pass because `hasattr(None, "files")` is false, hiding potential regressions.
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%2F38360&comment_hash=76467c0b9bc8444ec32753c8ea00d88b9bb8837cdd770c846ac7e5c3af16eef0&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38360&comment_hash=76467c0b9bc8444ec32753c8ea00d88b9bb8837cdd770c846ac7e5c3af16eef0&reaction=dislike'>👎</a>
##########
tests/unit_tests/extensions/test_types.py:
##########
@@ -246,11 +251,14 @@ def test_manifest_backend_no_files_field():
def test_extension_config_backend_defaults():
"""Test ExtensionConfigBackend has correct defaults."""
backend = ExtensionConfigBackend.model_validate({})
- assert backend.entryPoints == []
assert backend.files == []
-def test_manifest_backend_defaults():
- """Test ManifestBackend has correct defaults."""
- backend = ManifestBackend.model_validate({})
- assert backend.entryPoints == []
+def test_manifest_backend_required_entrypoint():
+ """Test ManifestBackend requires entrypoint field."""
+ backend = ManifestBackend.model_validate(
+ {"entrypoint": "superset_extensions.test_org.test_extension.entrypoint"}
+ )
+ assert (
+ backend.entrypoint == "superset_extensions.test_org.test_extension.entrypoint"
+ )
Review Comment:
**Suggestion:** The test claiming to verify that the backend entrypoint field is required never actually checks the error path (missing entrypoint), so it will still pass even if `ManifestBackend` stops enforcing this requirement, weakening test coverage and allowing regressions to slip through. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ ManifestBackend entrypoint requirement regression may go undetected.
- ⚠️ Weakens safety of extension backend loading tests.
```
</details>
```suggestion
with pytest.raises(ValidationError) as exc_info:
ManifestBackend.model_validate({})
assert "entrypoint" in str(exc_info.value)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run `pytest
tests/unit_tests/extensions/test_types.py::test_manifest_backend_required_entrypoint`.
2. Observe the test in `tests/unit_tests/extensions/test_types.py:257-264` only calls
`ManifestBackend.model_validate` with a valid `entrypoint` and asserts equality.
3. Note the docstring "Test ManifestBackend requires entrypoint field." contradicts the
implementation, which never exercises the missing-entrypoint case.
4. Because the test never validates behavior when `entrypoint` is absent, any future
change that makes `ManifestBackend.entrypoint` optional would not cause this test to fail,
weakening regression protection around the required-entrypoint contract.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/extensions/test_types.py
**Line:** 259:264
**Comment:**
*Logic Error: The test claiming to verify that the backend entrypoint field is required never actually checks the error path (missing entrypoint), so it will still pass even if `ManifestBackend` stops enforcing this requirement, weakening test coverage and allowing regressions to slip through.
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%2F38360&comment_hash=83d2964bb654aa197c6d6420c7d95b9b81b9af5359ab07487e0c3902d0a1d9c3&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38360&comment_hash=83d2964bb654aa197c6d6420c7d95b9b81b9af5359ab07487e0c3902d0a1d9c3&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on PR #38360:
URL: https://github.com/apache/superset/pull/38360#issuecomment-3992367835
> @villebro Previously, we accepted multiple entry points and now it's a single one. Is this intended? I thought entry points would just be another contribution type like tools, APIs, etc and you would just need to annotate a file instead of a convention.
@michael-s-molina Yes, this is intentional. The single entrypoint mirrors our frontend module federation pattern where `index.tsx` serves as the main entry point. So on the frontend we have a convention of `index.tsx`, while on the backend we have `entrypoint.py`.
IMO, an entrypoint isn't a contribution type like APIs or tools - it's infrastructure that simply registers contributions or triggers arbitrary code you wish to execute during bootstrap time. The frontend doesn't have "entrypoint contributions" either, and multiple entrypoints felt redundant since you can simply import multiple modules within a single entrypoint.
TL;DR: the proposed design here harmonizes backend/frontend architecture while maintaining full flexibility through imports, and aims to minimize developer overhead.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #38360:
URL: https://github.com/apache/superset/pull/38360#issuecomment-3990355707
@villebro Previously, we accepted multiple entry points and now it's a single one. Is this intended? I thought entry points would just be another contribution type like tools, APIs, etc and you would just need to annotate a file instead of a convention.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #38360:
URL: https://github.com/apache/superset/pull/38360#issuecomment-3989752200
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/38360?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `70.37037%` with `16 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 64.48%. Comparing base ([`1bd0546`](https://app.codecov.io/gh/apache/superset/commit/1bd054684edc8459d0fdc1f7cfa8330f505fbcfd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`825af7f`](https://app.codecov.io/gh/apache/superset/commit/825af7f2f0fa345878f7b95618bb3561644dd7a3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/38360?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...-extensions-cli/src/superset\_extensions\_cli/cli.py](https://app.codecov.io/gh/apache/superset/pull/38360?src=pr&el=tree&filepath=superset-extensions-cli%2Fsrc%2Fsuperset_extensions_cli%2Fcli.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZXh0ZW5zaW9ucy1jbGkvc3JjL3N1cGVyc2V0X2V4dGVuc2lvbnNfY2xpL2NsaS5weQ==) | 70.37% | [16 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38360?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
:x: Your project check has failed because the head coverage (96.03%) is below the target coverage (100.00%). You can increase the head coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #38360 +/- ##
==========================================
+ Coverage 64.29% 64.48% +0.18%
==========================================
Files 1811 1816 +5
Lines 71491 72015 +524
Branches 22775 22775
==========================================
+ Hits 45965 46436 +471
- Misses 25526 25579 +53
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/38360/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [superset-extensions-cli](https://app.codecov.io/gh/apache/superset/pull/38360/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.88% <70.37%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/38360?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2877121122
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -217,17 +218,36 @@ def copy_frontend_dist(cwd: Path) -> str:
def copy_backend_files(cwd: Path) -> None:
+ """Copy backend files based on pyproject.toml build configuration (validation already passed)."""
dist_dir = cwd / "dist"
- extension = read_json(cwd / "extension.json")
- if not extension:
- click.secho("❌ No extension.json file found.", err=True, fg="red")
- sys.exit(1)
+ backend_dir = cwd / "backend"
- for pat in extension.get("backend", {}).get("files", []):
- for f in cwd.glob(pat):
+ # Read build config from pyproject.toml
+ pyproject = read_toml(backend_dir / "pyproject.toml")
+ assert pyproject
Review Comment:
**Suggestion:** Using a bare `assert pyproject` after reading `backend/pyproject.toml` can raise an unhandled AssertionError (for example when the file is missing or unreadable), causing the CLI to crash with a stack trace instead of showing a clear error message, especially in `dev` mode where no prior validation is performed. Replace the assertion with an explicit check that prints a helpful error via `click.secho` and exits with a non-zero status using `sys.exit(1)` so that misconfigured or missing `pyproject.toml` for the backend is handled consistently and gracefully. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ `dev` watch command crashes on invalid backend pyproject.toml.
- ⚠️ Extension developers see raw AssertionError stack trace.
- ⚠️ Watch mode unusable until config issue diagnosed manually.
```
</details>
```suggestion
if not pyproject:
click.secho(
"❌ Failed to read backend pyproject.toml",
err=True,
fg="red",
)
sys.exit(1)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In an extension project, ensure a `backend/` directory exists but make
`backend/pyproject.toml` unreadable or invalid so that `read_toml()` returns a falsy value
(e.g., delete the file or empty it). This is the directory inspected in
`superset-extensions-cli/src/superset_extensions_cli/cli.py` by `copy_backend_files()` at
lines 220–252.
2. From that project root, run the CLI `dev` command (`superset-extensions-cli dev`),
which maps to the `dev()` function in
`superset-extensions-cli/src/superset_extensions_cli/cli.py` (defined after the `bundle()`
and `build()` commands) and is the primary watch-mode entry point for extension
development.
3. Inside `dev()`, when `backend_dir = cwd / "backend"` exists, it calls
`rebuild_backend(cwd)`, which in turn calls `copy_backend_files(cwd)` (same file,
`rebuild_backend()` defined immediately after `rebuild_frontend()` and before the Click
commands).
4. `copy_backend_files()` executes `pyproject = read_toml(backend_dir / "pyproject.toml");
assert pyproject` at lines 226–227. Because `pyproject` is falsy, Python raises an
unhandled `AssertionError`, causing the `dev` command to crash with a stack trace instead
of a user-friendly `click.secho` error, unlike the `validate()` and `build()` paths which
explicitly check `pyproject` and exit cleanly.
```
</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:** 227:227
**Comment:**
*Logic Error: Using a bare `assert pyproject` after reading `backend/pyproject.toml` can raise an unhandled AssertionError (for example when the file is missing or unreadable), causing the CLI to crash with a stack trace instead of showing a clear error message, especially in `dev` mode where no prior validation is performed. Replace the assertion with an explicit check that prints a helpful error via `click.secho` and exits with a non-zero status using `sys.exit(1)` so that misconfigured or missing `pyproject.toml` for the backend is handled consistently and gracefully.
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%2F38360&comment_hash=5df94a1f35551c2b4f0af6c839454e8695dc2522da1ed8e8091b07540943875b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38360&comment_hash=5df94a1f35551c2b4f0af6c839454e8695dc2522da1ed8e8091b07540943875b&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38360:
URL: https://github.com/apache/superset/pull/38360#issuecomment-3990178548
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38360:
URL: https://github.com/apache/superset/pull/38360#issuecomment-3992662528
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2879453144
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -217,17 +218,36 @@ def copy_frontend_dist(cwd: Path) -> str:
def copy_backend_files(cwd: Path) -> None:
+ """Copy backend files based on pyproject.toml build configuration (validation already passed)."""
dist_dir = cwd / "dist"
- extension = read_json(cwd / "extension.json")
- if not extension:
- click.secho("❌ No extension.json file found.", err=True, fg="red")
- sys.exit(1)
+ backend_dir = cwd / "backend"
- for pat in extension.get("backend", {}).get("files", []):
- for f in cwd.glob(pat):
+ # Read build config from pyproject.toml
+ pyproject = read_toml(backend_dir / "pyproject.toml")
+ assert pyproject
Review Comment:
Not relevant - the assertion there is only to satisfy mypy
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #38360:
URL: https://github.com/apache/superset/pull/38360#discussion_r2879473467
##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -217,17 +222,36 @@ def copy_frontend_dist(cwd: Path) -> str:
def copy_backend_files(cwd: Path) -> None:
+ """Copy backend files based on pyproject.toml build configuration (validation already passed)."""
dist_dir = cwd / "dist"
- extension = read_json(cwd / "extension.json")
- if not extension:
- click.secho("❌ No extension.json file found.", err=True, fg="red")
- sys.exit(1)
+ backend_dir = cwd / "backend"
- for pat in extension.get("backend", {}).get("files", []):
- for f in cwd.glob(pat):
+ # Read build config from pyproject.toml
+ pyproject = read_toml(backend_dir / "pyproject.toml")
+ assert pyproject
+ build_config = (
+ pyproject.get("tool", {}).get("apache_superset_extensions", {}).get("build", {})
+ )
+ include_patterns = build_config.get("include", [])
+ exclude_patterns = build_config.get("exclude", [])
+
+ # Process include patterns
+ for pattern in include_patterns:
Review Comment:
Good catch 👍
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro merged PR #38360:
URL: https://github.com/apache/superset/pull/38360
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore(extensions): clean up backend entrypoints and file globs [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #38360:
URL: https://github.com/apache/superset/pull/38360#issuecomment-3989751744
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 825af7f2f0fa345878f7b95618bb3561644dd7a3 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69a6a9352f2c9b0009a5f43c |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-38360--superset-docs-preview.netlify.app](https://deploy-preview-38360--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-38360--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org