amoghrajesh commented on code in PR #71212:
URL: https://github.com/apache/airflow/pull/71212#discussion_r3736072580
##########
scripts/ci/prek/check_sdk_imports_in_core.py:
##########
@@ -42,23 +78,75 @@ def check_file_for_sdk_imports(file_path: Path) ->
list[tuple[int, str]]:
)
-def main():
- parser = argparse.ArgumentParser(description="Check for SDK imports in
airflow-core files")
- parser.add_argument("files", nargs="*", help="Files to check")
- args = parser.parse_args()
+class SdkImportsAllowlistManager(AllowlistManager):
+ def __init__(self, allowlist_file: Path) -> None:
+ super().__init__(allowlist_file, repo_root=REPO_ROOT)
- if not args.files:
- return
+ def iter_files(self) -> Iterable[Path]:
+ return CORE_SRC_ROOT.rglob("*.py")
- report_import_violations(
- args.files,
- check_func=check_file_for_sdk_imports,
- violation_label="SDK import(s) in core files",
- nocheck_code=NOCHECK_CODE,
- only_python_files=True,
+ def count_occurrences(self, path: Path) -> int:
+ return len(check_file_for_sdk_imports(path))
+
+ def violation_panel_text(self) -> str:
+ return (
+ "New [bold]airflow.sdk[/bold] import detected in airflow-core.\n"
+ "Core (scheduler/API server) should not gain new runtime
dependencies "
+ "on the Task SDK.\n"
+ "If this import is a genuine one-off, append `# noqa: SDK001` to
the "
+ "import line.\n"
+ "If it's an intentional, broader exception, run:\n\n"
+ " [cyan]uv run ./scripts/ci/prek/check_sdk_imports_in_core.py
--generate[/cyan]\n\n"
+ "to regenerate the allowlist, then commit the updated\n"
+ "[cyan]generated/known_sdk_imports_in_core.txt[/cyan]."
+ )
Review Comment:
It already mentioned noqa, but you are right that it never said which route
to pick, which is the actual question. Reworded the panel to lead with "drop
the import if you can", then two numbered options: `# noqa: SDK001` for a
single deliberate import line, which stays out of the count and out of the
allowlist entirely, versus `--generate FILE` for a file that genuinely needs
several, which raises that files recorded ceiling. Same distinction now in the
module docstring.
--
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]
