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: [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]
