Pushkal-Gupta commented on code in PR #74654:
URL: https://github.com/apache/airflow/pull/74654#discussion_r4280470883
##########
scripts/ci/prek/extract_permissions.py:
##########
@@ -276,8 +279,11 @@ def extract_from_file(path: pathlib.Path) ->
list[PermissionEntry]:
source = path.read_text(encoding="utf-8")
tree = ast.parse(source)
except (OSError, SyntaxError) as exc:
- print(f"[WARN] Could not parse {path.name}: {exc}", file=sys.stderr)
- return []
+ # Returning no entries here would quietly shrink a security reference
that readers
+ # trust to be complete, and the caller has no way to tell that apart
from a file
+ # that genuinely declares no routes. Stop the run instead.
+ print(f"[FAIL] Could not parse {path}: {exc}", file=sys.stderr)
+ raise SystemExit(1)
Review Comment:
Makes sense, done. `extract_from_file` raises `RouteParseError` now and both
`main()`s catch it, print `[FAIL] ...` and return 1, same as the other failures
in there.
It also forced me to handle it in the FAB script's `main()` explicitly,
which is better anyway, before that script was just inheriting the exit through
the import without saying so anywhere.
--
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]
