Skip to content

Fix airflowctl config lint filters matching partial config names - #75690

Merged
shahar1 merged 3 commits into
apache:mainfrom
rjgoyln:fix-airflowctl-config-lint-filters
Oct 27, 2026
Merged

shahar1 merged 3 commits into
apache:mainfrom
rjgoyln:fix-airflowctl-config-lint-filters

Conversation

@rjgoyln

@rjgoyln rjgoyln commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Continues #70940, closed by the open PR limit.

Summary

airflowctl config lint parses --section, --option, --ignore-section and --ignore-option as plain strings, so each filter matches any config name that is a substring of its value:

  • --option auth_backends also reports auth_backend.
  • --ignore-option auth_backends also hides the auth_backend warning.
  • --ignore-section api --verbose prints Ignored sections: a, p, i.

Five option names in CONFIGS_CHANGES are substrings of another. The comma-separated lists the docstring documents only worked through the same substring match.

Parsing all four with string_list_type matches airflow-core's airflow config lint.


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

Generated-by: Claude Code (Opus 5.5) following the guidelines

The lint filters were declared as plain strings, so the membership tests in
the command body compared a name against a string rather than against a list.
Any name that happened to be a prefix of what the user typed slipped through:
`--option auth_backends` also reported `auth_backend`, and the mirror case on
`--ignore-option` silently suppressed warnings the user still wanted to see.
Five such pairs exist in CONFIGS_CHANGES today.

The comma-separated lists the docstring has always advertised only appeared to
work because substring matching happened to accept them; the verbose output
joined the same strings character by character.

airflow-core already parses these four arguments with string_list_type, which
was defined here but left unused.
The fixture comment called every collision a prefix match and cited
sql_alchemy_conn/sql_alchemy_connect_args, a pair the fixture never
contains, so it said nothing true about what each entry is for. The
other edits bring the new tests in line with the rest of the file and
with the testing standards, which ask for autospec on mocks.
@shahar1

shahar1 commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Merged from main, please let it run and I'll review afterwards :)

@rjgoyln

rjgoyln commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor Author

OK. Thanks!

@shahar1
shahar1 merged commit 91fbcbf into apache:main Oct 27, 2026
87 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: airflow-ctl/v0-1-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
✅ airflow-ctl/v0-1-test PR Link

shahar1 pushed a commit that referenced this pull request Oct 27, 2026
…artial config names (#75690) (#75736)

The lint filters were declared as plain strings, so the membership tests in
the command body compared a name against a string rather than against a list.
Any name that happened to be a prefix of what the user typed slipped through:
`--option auth_backends` also reported `auth_backend`, and the mirror case on
`--ignore-option` silently suppressed warnings the user still wanted to see.
Five such pairs exist in CONFIGS_CHANGES today.

The comma-separated lists the docstring has always advertised only appeared to
work because substring matching happened to accept them; the verbose output
joined the same strings character by character.

airflow-core already parses these four arguments with string_list_type, which
was defined here but left unused.
(cherry picked from commit 91fbcbf)

Co-authored-by: rjgoyln <151457491+rjgoyln@users.noreply.github.com>
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