Skip to content

Use ValueError for unsupported recursive glob in ADLS upload - #70540

Merged
potiuk merged 1 commit into
apache:mainfrom
mitre88:narrow-local-to-adls-exception
Jul 31, 2026
Merged

potiuk merged 1 commit into
apache:mainfrom
mitre88:narrow-local-to-adls-exception

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.

LocalFilesystemToADLSOperator.execute raised AirflowException when local_path contains an unsupported ** recursive glob — a plain input-validation error, now a ValueError. The known_airflow_exceptions.txt entry for the file drops from 1 to 0, and the existing test asserts the new type.


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

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

An unsupported '**' glob in local_path is an input-validation error, so
it should not raise 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 by triage when all criteria pass, or by the fork CI reconciler when fork CI is green. 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 — textbook version of this migration. An unsupported glob pattern is a caller-supplied argument error, which is exactly what ValueError is for, and the message is unchanged so nothing user-facing shifts except the type.

Three things done right that often get missed in this campaign: the now-unused AirflowException import is removed rather than left dangling; the known_airflow_exceptions.txt entry is deleted outright (count 1 -> 0) rather than decremented to a stale zero; and the test gains a match= argument, so it now pins the message as well as the type — strictly stronger than before.

Worth noting for completeness: ValueError isn't a subclass of AirflowException, so a Dag wrapping this operator in except AirflowException stops catching it. That's inherent to the migration rather than anything wrong here, and the ratchet file exists precisely to track it — just flagging it's a real behaviour change, not a pure refactor.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:microsoft-azure Azure-related issues ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants