Skip to content

Reject allowed_tables=None in SQLToolset so only omitting it allows every table - #73452

Merged
kaxil merged 1 commit into
apache:mainfrom
astronomer:sqltoolset-allowed-tables-sentinel
Sep 21, 2026
Merged

kaxil merged 1 commit into
apache:mainfrom
astronomer:sqltoolset-allowed-tables-sentinel

Conversation

@kaxil

@kaxil kaxil commented Sep 21, 2026

Copy link
Copy Markdown
Member

Follow-up to #73381, which made SQLToolset(allowed_tables=[]) raise instead of exposing every table. allowed_tables=None still had that effect, and it is the more common shape for a list that "resolves to nothing": Variable.get("tables", default_var=None), a dict.get(), a JSON null from a config file. The guardrail #73381 added therefore still switched itself off for exactly the Dags it was meant to protect.

allowed_tables now defaults to a private sentinel instead of None. Omitting the argument keeps the old allow-all behaviour, but no value requests it any more: None and [] both raise ValueError at construction. A runtime lookup that comes back empty can only make the toolset fail at Dag import, never widen it to the whole schema. Exposing every table stays possible, but only as a static decision visible in the Dag file.

Why a sentinel rather than making allowed_tables required or adding an allow_all_tables flag. Allow-all is a legitimate default for a scratch or read-only database and every example Dag relies on it, so requiring the argument would break the common case for no safety gain. A separate opt-in flag adds surface for something the absence of the argument already expresses. The sentinel is the same _UNSET = object() pattern the provider's pydantic-ai hook already uses to tell "not passed" from an explicit None.

Because #73381 is unreleased, its changelog note is rewritten in place to cover both values rather than adding a second note. Dags that passed allowed_tables=None explicitly should drop the argument; nothing else changes. Common AI is still 0.x.


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@kaxil
kaxil merged commit 9100ca8 into apache:main Sep 21, 2026
83 checks passed
@kaxil
kaxil deleted the sqltoolset-allowed-tables-sentinel branch September 21, 2026 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants