Fix ObjectStoragePath NoCredentialsError when using conn_id with remote stores#64634
Conversation
…d fs into __wrapped__._fs_cached
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
611e74b
Compare
There 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). |
…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.