Skip to content

handle azure Gen2 storage path when parsing ref#7339

Merged
SVilgelm merged 2 commits intomainfrom
svilgelm/handle-abfs
May 5, 2026
Merged

handle azure Gen2 storage path when parsing ref#7339
SVilgelm merged 2 commits intomainfrom
svilgelm/handle-abfs

Conversation

@SVilgelm
Copy link
Copy Markdown
Contributor

@SVilgelm SVilgelm commented May 5, 2026

Summary

  • Cherry-picks the ABFS Gen2 path-parsing fix from the legacy flytestdlib repo (original PR: https://github.com/flyteorg/flytestdlib/pull/841) onto main.
  • Fixes DataReference.Split for Azure ADLS Gen2 (abfs://container@account.dfs.core.windows.net/path) URLs, where url.Parse puts the container in the User field instead of Host.
  • Scopes the u.User.Username() override to abfs/abfss so s3://accessKey:secret@bucket/key-style URLs still return bucket as the container.
  • Adds a regression test for the S3-with-userinfo case and an abfss test.

Test plan

  • go test ./flytestdlib/storage/... -run TestDataReference

🤖 Generated with Claude Code

Signed-off-by: Paul Dittamo <pvdittamo@gmail.com>
Copilot AI review requested due to automatic review settings May 5, 2026 01:44
@github-actions github-actions Bot added the flyte2 label May 5, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates flytestdlib/storage to correctly split Azure ADLS Gen2 (ABFS) URLs where net/url.Parse places the filesystem/container name in URL.User due to the container@host authority format.

Changes:

  • Update DataReference.Split() to use u.User.Username() as the container when present (to support abfs://container@account.dfs.core.windows.net/...).
  • Expand TestDataReference_Split with ABFS (with/without path) and GCS subtests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
flytestdlib/storage/storage.go Adjusts DataReference.Split() to extract the container from URL.User when @ is present in the authority (ABFS Gen2 format).
flytestdlib/storage/storage_test.go Adds subtests covering ABFS Gen2 parsing and adds additional scheme coverage (GCS).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread flytestdlib/storage/storage.go Outdated
Comment thread flytestdlib/storage/storage_test.go
Comment thread flytestdlib/storage/storage.go Outdated
pvditt
pvditt previously approved these changes May 5, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@SVilgelm SVilgelm merged commit 1484985 into main May 5, 2026
21 checks passed
@SVilgelm SVilgelm deleted the svilgelm/handle-abfs branch May 5, 2026 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants