Skip to content

Use built-in exceptions for Postgres hook input validation - #70537

Merged
potiuk merged 1 commit into
apache:mainfrom
mitre88:narrow-postgres-hook-exceptions
Jul 31, 2026
Merged

Use built-in exceptions for Postgres hook input validation#70537
potiuk merged 1 commit into
apache:mainfrom
mitre88:narrow-postgres-hook-exceptions

Conversation

@mitre88

@mitre88 mitre88 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Part of the ongoing clean-up of broad AirflowException usages (enforced by the check-no-new-airflow-exceptions ratchet), following the pattern of #66279.

PostgresHook raised AirflowException for two input-validation failures: a sqlalchemy_query connection extra that is not a dict (now TypeError), and requesting realdictcursor with psycopg3 (now ValueError, matching the invalid-cursor branch immediately below it that already raises ValueError). The known_airflow_exceptions.txt entry for the file drops from 2 to 0, and the existing test now asserts the specific exception type and message.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Fable 5)

Generated-by: Claude Code (Fable 5) following the guidelines

A wrong type for sqlalchemy_query is a TypeError, and requesting the
psycopg3-unsupported realdictcursor is a ValueError like the invalid
cursor branch right next to it; both were raised as the broad
AirflowException the community is actively reducing. Follows the
clean-up pattern of apache#66279.
@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jul 28, 2026

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — and the type selection here is better than most of this migration.

Splitting the two raises by kind rather than defaulting both to ValueError is the right call: sqlalchemy_query not being a dict is a type error, while realdictcursor is a valid-type-but-unsupported value. That distinction usually gets flattened. The realdictcursor change also now matches its immediate neighbour, which already raises ValueError(f"Invalid cursor passed {_cursor}...") — so the two adjacent failure paths finally agree.

Removing the ratchet entry outright is right too: it was ::2 and both occurrences are gone, so the line goes rather than being decremented to a stale count. The match= on the updated test is a genuine strengthening.

One small gap: the realdictcursor branch has no test asserting the new type. The only realdict reference in test_postgres.py is a monkeypatch.setattr(..., "RealDictCursor", None) at line 69 — nothing exercises the raise. So half this PR is covered and half isn't. It's a message-preserving type change so the risk is low, and a two-line pytest.raises(ValueError, match="realdictcursor is not supported") around hook._get_cursor("realdictcursor") would close it whenever you next touch the file. Not blocking.


Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

@potiuk
potiuk merged commit e80a6bc into apache:main Jul 31, 2026
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants