You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/05/20 19:01:18 UTC
[PR] ci(lint): enforce no function-body imports (PLC0415) [superset]
rusackas opened a new pull request, #40296:
URL: https://github.com/apache/superset/pull/40296
### SUMMARY
Follow-up to [#40231](https://github.com/apache/superset/pull/40231) (merged) — a reviewer flagged a `from datetime import datetime, timedelta` landing inside a test method body rather than at the top of the file. A lint rule catches that everywhere.
Adds a dedicated **`ruff-import-placement`** pre-commit hook running `ruff check --select PLC0415 --preview --no-fix`. Any new function-body import fails CI.
**Why a separate hook instead of `[tool.ruff.lint] select`:** `PLC0415` is preview-only in ruff today. Flipping `preview = true` in the main config would also activate behavior changes for unrelated stable rules (`F841`, `I001`, etc.) and produce ~500 spurious lints. A dedicated hook isolates the change.
### Grandfathered violations
`ruff check --select PLC0415 --preview --add-noqa` discovered **2,657 existing violations across ~410 files** and annotated each with a per-line `# noqa: PLC0415`. Many are intentional:
- Circular-import workarounds (e.g. `superset/app.py`)
- Lazy-loads (heavy modules like pandas, sqlglot subpackages)
- Feature-flag-guarded optional dependencies
- Test files importing test-only helpers
Moving them to the top of their files would break things. The noqa is the safe boundary: today's behavior is preserved, new violations are blocked.
### Going forward
New code that legitimately needs a function-body import must add `# noqa: PLC0415` with a justification.
### BEFORE/AFTER
| | Before | After |
|---|---|---|
| New function-body import in a PR | Silently accepted | Fails `ruff-import-placement` hook |
| Existing function-body imports | Untracked | Marked `# noqa: PLC0415` |
| Main ruff hook | Unchanged | Unchanged |
| Other preview rules | Disabled | Still disabled |
### Also in this PR
Silences a pre-existing `C901` ("too complex") on `mcp_service/sql_lab/tool/execute_sql.py::execute_sql` that wasn't firing under CI's pinned ruff 0.9.7 but does fire under newer local ruff. Unrelated to import-placement; touched as a side-effect of running the full pre-commit suite locally.
### TESTING INSTRUCTIONS
\`\`\`bash
# Should pass cleanly
pre-commit run ruff-import-placement --all-files
# Should also pass
pre-commit run ruff --all-files
\`\`\`
To prove the gate works, add a new function-body import without a noqa and watch the hook fail.
### ADDITIONAL INFORMATION
- [ ] Has associated issue
- [ ] Required feature flags
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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] ci(lint): enforce no function-body imports (PLC0415) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #40296: ci(lint): enforce no function-body imports (PLC0415)
URL: https://github.com/apache/superset/pull/40296
--
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] ci(lint): enforce no function-body imports (PLC0415) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40296:
URL: https://github.com/apache/superset/pull/40296#issuecomment-4516378037
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] ci(lint): enforce no function-body imports (PLC0415) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40296:
URL: https://github.com/apache/superset/pull/40296#issuecomment-4502596887
This attempt didn't really FIX much... just makes a lot of noise. Closing.
--
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] ci(lint): enforce no function-body imports (PLC0415) [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40296:
URL: https://github.com/apache/superset/pull/40296#issuecomment-4501964051
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40296?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 `35.15924%` with `509 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 63.43%. Comparing base ([`b23c65e`](https://app.codecov.io/gh/apache/superset/commit/b23c65e04f155da3e9c0c8f6f79aa5da0f830220?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d0c5eec`](https://app.codecov.io/gh/apache/superset/commit/d0c5eece5a2c516936ccefa289c82014c3614d30?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40296?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/sql/execution/executor.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fsql%2Fexecution%2Fexecutor.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc3FsL2V4ZWN1dGlvbi9leGVjdXRvci5weQ==) | 0.00% | [29 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/chart/tool/generate\_chart.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fchart%2Ftool%2Fgenerate_chart.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY2hhcnQvdG9vbC9nZW5lcmF0ZV9jaGFydC5weQ==) | 0.00% | [22 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...perset/mcp\_service/chart/tool/get\_chart\_preview.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fchart%2Ftool%2Fget_chart_preview.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY2hhcnQvdG9vbC9nZXRfY2hhcnRfcHJldmlldy5weQ==) | 0.00% | [18 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/chart/tool/get\_chart\_data.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fchart%2Ftool%2Fget_chart_data.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY2hhcnQvdG9vbC9nZXRfY2hhcnRfZGF0YS5weQ==) | 0.00% | [17 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/auth.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fauth.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvYXV0aC5weQ==) | 27.27% | [16 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/chart/chart\_utils.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fchart%2Fchart_utils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY2hhcnQvY2hhcnRfdXRpbHMucHk=) | 0.00% | [14 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/middleware.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fmiddleware.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvbWlkZGxld2FyZS5weQ==) | 0.00% | [14 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/server.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fserver.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2Uvc2VydmVyLnB5) | 0.00% | [14 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/mcp\_service/chart/tool/get\_chart\_sql.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fchart%2Ftool%2Fget_chart_sql.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY2hhcnQvdG9vbC9nZXRfY2hhcnRfc3FsLnB5) | 0.00% | [13 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../dashboard/tool/add\_chart\_to\_existing\_dashboard.py](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fdashboard%2Ftool%2Fadd_chart_to_existing_dashboard.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvZGFzaGJvYXJkL3Rvb2wvYWRkX2NoYXJ0X3RvX2V4aXN0aW5nX2Rhc2hib2FyZC5weQ==) | 0.00% | [13 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [115 more](https://app.codecov.io/gh/apache/superset/pull/40296?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40296 +/- ##
==========================================
- Coverage 64.18% 63.43% -0.76%
==========================================
Files 2591 2591
Lines 138446 138446
Branches 32116 32116
==========================================
- Hits 88867 87818 -1049
- Misses 48048 49091 +1043
- Partials 1531 1537 +6
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40296/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/40296/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.41% <19.87%> (ø)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/40296/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/40296/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/40296/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.09% <20.25%> (ø)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/40296/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.91% <35.15%> (-1.67%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/40296/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.78% <34.52%> (ø)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/40296/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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/40296?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] ci(lint): enforce no function-body imports (PLC0415) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40296:
URL: https://github.com/apache/superset/pull/40296#issuecomment-4501687380
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] ci(lint): enforce no function-body imports (PLC0415) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40296:
URL: https://github.com/apache/superset/pull/40296#discussion_r3287020271
##########
superset/models/core.py:
##########
@@ -1317,7 +1317,7 @@ def execute(
:param options: QueryOptions with execution settings
:returns: QueryResult with status, data, and metadata
"""
- from superset.sql.execution import SQLExecutor
+ from superset.sql.execution import SQLExecutor # noqa: PLC0415
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>PLC0415 noqa masks violation</b></div>
<div id="fix">
The `# noqa: PLC0415` comment masks a linting violation. Per pyproject.toml lines 404-428, `superset/models/` is not in the list of directories allowed to have function-body imports. The proper fix is to move `from superset.sql.execution import SQLExecutor` to the top-level imports section, not suppress the error.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/models/core.py (lines 85-86) ---
85: from superset.utils import cache as cache_util, core as utils, json
86: +from superset.sql.execution import SQLExecutor
87: from superset.utils.backports import StrEnum
--- superset/models/core.py (lines 1318-1322) ---
1318: :returns: QueryResult with status, data, and metadata
1319: """
1320: - from superset.sql.execution import SQLExecutor # noqa: PLC0415
1321: + return SQLExecutor(self).execute(sql, options)
1322:
--- superset/models/core.py (lines 1334-1338) ---
1334: :returns: AsyncQueryHandle for tracking the query
1335: """
1336: - from superset.sql.execution import SQLExecutor # noqa: PLC0415
1337: + return SQLExecutor(self).execute_async(sql, options)
```
</div>
</details>
</div>
<small><i>Code Review Run #86bbd8</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_cli_build.py:
##########
@@ -347,7 +347,7 @@ def test_build_manifest_exits_when_extension_json_missing(isolated_filesystem):
@pytest.mark.unit
def test_clean_dist_frontend_removes_frontend_dist(isolated_filesystem):
"""Test clean_dist_frontend removes frontend/dist directory specifically."""
- from superset_extensions_cli.cli import clean_dist_frontend
+ from superset_extensions_cli.cli import clean_dist_frontend # noqa: PLC0415
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>PLC0415 suppression masks fixable issue</b></div>
<div id="fix">
The `# noqa: PLC0415` comments suppress a legitimate pylint warning. These 4 functions should be added to the existing top-level import block (lines 24-31) instead of using local imports. Having imports in two places creates maintenance divergence risk.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset-extensions-cli/tests/test_cli_build.py (lines 24-31) ---
24: from superset_extensions_cli.cli import (
25: app,
26: build_manifest,
27: clean_dist,
28: copy_backend_files,
29: copy_frontend_dist,
30: init_frontend_deps,
31: + clean_dist_frontend,
32: + run_frontend_build,
33: + rebuild_frontend,
34: + rebuild_backend,
35: )
--- superset-extensions-cli/tests/test_cli_build.py (lines 347-353) ---
347: @pytest.mark.unit
348: def test_clean_dist_frontend_removes_frontend_dist(isolated_filesystem):
349: """Test clean_dist_frontend removes frontend/dist directory specifically."""
350: - from superset_extensions_cli.cli import clean_dist_frontend # noqa: PLC0415
351: +
352: # Create dist/frontend structure
353: dist_dir = isolated_filesystem / "dist"
--- superset-extensions-cli/tests/test_cli_build.py (lines 366-372) ---
366: @pytest.mark.unit
367: def test_clean_dist_frontend_handles_nonexistent_directory(isolated_filesystem):
368: """Test clean_dist_frontend handles case where frontend dist doesn't exist."""
369: - from superset_extensions_cli.cli import clean_dist_frontend # noqa: PLC0415
370: +
371: # No dist directory exists
372: clean_dist_frontend(isolated_filesystem)
--- superset-extensions-cli/tests/test_cli_build.py (lines 377-383) ---
377: @pytest.mark.unit
378: def test_run_frontend_build_with_output_messages(isolated_filesystem):
379: """Test run_frontend_build produces expected output messages."""
380: - from superset_extensions_cli.cli import run_frontend_build # noqa: PLC0415
381: +
382: frontend_dir = isolated_filesystem / "frontend"
383: frontend_dir.mkdir()
--- superset-extensions-cli/tests/test_cli_build.py (lines 406-412) ---
406: isolated_filesystem, return_code, expected_result
407: ):
408: """Test rebuild_frontend handles different build results."""
409: - from superset_extensions_cli.cli import rebuild_frontend # noqa: PLC0415
410: +
411: # Create frontend structure
412: frontend_dir = isolated_filesystem / "frontend"
--- superset-extensions-cli/tests/test_cli_build.py (lines 434-440) ---
434: @pytest.mark.unit
435: def test_rebuild_backend_calls_copy_and_shows_message(isolated_filesystem):
436: """Test rebuild_backend calls copy_backend_files and shows success message."""
437: - from superset_extensions_cli.cli import rebuild_backend # noqa: PLC0415
438: +
439: # Create extension.json
440: extension_json = {
```
</div>
</details>
</div>
<small><i>Code Review Run #86bbd8</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/utils/rls.py:
##########
@@ -84,7 +84,7 @@ def get_predicates_for_table(
table must be fully qualified, with catalog (null if the DB doesn't support) and
schema.
"""
- from superset.connectors.sqla.models import SqlaTable
+ from superset.connectors.sqla.models import SqlaTable # noqa: PLC0415
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>PLC0415 noqa missing justification</b></div>
<div id="fix">
The pre-commit hook for PLC0415 explicitly requires a justification when adding `# noqa: PLC0415`. The policy states: 'new code must either move the import to the top or add the same noqa with a justification.' Without justification, future maintainers won't understand why this import is deferred.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/utils/rls.py (lines 84-90)---
84: table must be fully qualified, with catalog (null if the DB doesn't support) and
85: schema.
86: """
87: - from superset.connectors.sqla.models import SqlaTable # noqa: PLC0415
87: + from superset.connectors.sqla.models import SqlaTable # noqa: PLC0415 - avoid circular import with database models at module load time
88:
89: # if the dataset in the RLS has null catalog, match it when using the default
90: # catalog
```
</div>
</details>
</div>
<small><i>Code Review Run #86bbd8</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/utils/rls.py:
##########
@@ -158,7 +158,7 @@ def collect_rls_predicates_for_sql(
(kept consistent with what's actually applied at query time).
:return: List of RLS predicate strings that would be applied
"""
- from superset.sql.parse import SQLScript
+ from superset.sql.parse import SQLScript # noqa: PLC0415
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>PLC0415 noqa missing justification</b></div>
<div id="fix">
The pre-commit hook for PLC0415 explicitly requires a justification when adding `# noqa: PLC0415`. The policy states: 'new code must either move the import to the top or add the same noqa with a justification.' Without justification, future maintainers won't understand why this import is deferred.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- superset/utils/rls.py (lines 158-164)---
158: (kept consistent with what's actually applied at query time).
159: :return: List of RLS predicate strings that would be applied
160: """
161: - from superset.sql.parse import SQLScript # noqa: PLC0415
161: + from superset.sql.parse import SQLScript # noqa: PLC0415 - SQLScript depends on database engine which may not be initialized at module load
162:
163: try:
164: parsed_script = SQLScript(sql, engine=database.db_engine_spec.engine)
```
</div>
</details>
</div>
<small><i>Code Review Run #86bbd8</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