Uh oh!
There was an error while loading. Please reload this page.
Fix ObjectStoragePath NoCredentialsError when using conn_id with remote stores - #64634
Conversation
…d fs into __wrapped__._fs_cached
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
pankajkoti
left a comment
There was a problem hiding this comment.
Thanks @vatsrahul1001 for the lightning-fast fix 🚀 . I applied and tested the patch, and my DAG is happy now ✅ .
It would be great if someone with deeper expertise could also review the PR for any nuances I might have missed.
39c165d to
611e74bCompareThere was a problem hiding this comment.
Pull request overview
Fixes ObjectStoragePath credential resolution regressions introduced by the move to ProxyUPath, ensuring delegated path operations use Airflow-attached/authenticated filesystems when conn_id is provided.
Changes:
- Injects the Airflow-authenticated filesystem into the wrapped
UPath’s_fs_cachedduringObjectStoragePath.__init__. - Adds a secondary injection path in
_from_upathfor wrapping “fresh”UPathinstances without a cached filesystem. - Adds regression tests covering delegated operations (e.g.
exists,mkdir,is_dir,touch,iterdir) withconn_id.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
task-sdk/src/airflow/sdk/io/path.py | Injects authenticated FS into __wrapped__._fs_cached in __init__ and _from_upath to fix delegated method behavior with conn_id. |
task-sdk/tests/task_sdk/io/test_path.py | Adds regression tests ensuring delegated filesystem operations use the Airflow-attached FS when conn_id is set (including propagation to children). |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…gging, fix test imports
Hi maintainer, this PR was merged without a milestone set.
|
Backport successfully created: v3-2-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
…id with remote stores (apache#64634) * Fix ObjectStoragePath credential resolution by injecting authenticated fs into __wrapped__._fs_cached (cherry picked from commit f391942) Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
Problem
After the migration from
CloudPathtoProxyUPath(#60519),ObjectStoragePathsilently breaks for any remote store when a
conn_idis provided.Root cause: ProxyUPath delegates all filesystem operations to an inner self.wrapped UPath instance. That instance is constructed with empty storage_options because conn_id is stripped before super().init() (per #62701 to prevent it leaking into fsspec). So self.wrapped.fs creates an unauthenticated s3fs/gcsfs — ignoring the conn_id entirely.
This affects every delegated method: exists, mkdir, is_dir, is_file, touch, unlink, rmdir, iterdir, glob, rglob, walk, rename, read_bytes, write_bytes, read_text, write_text, and open.
Reported by @pankajkoti in 3.2.0rc1 testing.
Fix
Inject the Airflow-authenticated filesystem directly into self.wrapped._fs_cached immediately after super().init().
UPath propagates _fs_cached from parent to child whenever new path segments are appended (p / "sub", parent, iterdir results, glob results, etc.), so two injection points cover every case:
init — for paths constructed directly
_from_upath — for paths wrapped from a fresh UPath with no cached fs
This fixes all delegated methods in 22 lines with no per-method overrides.
Testing
claude
{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.