Skip to content

Support account in az:// URIs (az://container@account/...) - #1887

Open
dreadatour wants to merge 6 commits into
mainfrom
az-account-name-2
Open

Support account in az:// URIs (az://container@account/...)#1887
dreadatour wants to merge 6 commits into
mainfrom
az-account-name-2

Conversation

@dreadatour

Copy link
Copy Markdown
Contributor

Implements #1859: embed the Azure storage account in the URI — az://container@account/path — so a single string identifies both the container and the account, and URIs pointing to different accounts can coexist in one run. Supersedes #1798, which threaded --account-name through the knowledge-skill scripts instead.

Design

The account is parsed once, at the AzureClient boundary, and stays in the client name: client.name, client.uri, and File.source keep container@account, so saved datasets round-trip the account with no side-channel config. adlfs already tolerates az://container@account/path in every path operation (_strip_protocol folds the netloc back into container/path), so the change is confined to:

  • AzureClient.__init__ — parse the netloc, inject account_name into fs kwargs (URI wins over client_config), keep self.container for container-level SDK calls (_fetch_flat)
  • AzureClient.from_source — preserve @account (base _strip_protocol would drop it)
  • AzureClient.bucket_status — accept container@account, enabling the anonymous-access probe without an extra flag

Listing dataset names need nothing: @ is already sanitized injectively (_x40) and round-trips.

Knowledge skill

  • source_to_https builds https://account.blob.core.windows.net/container from the container@account netloc (env AZURE_STORAGE_ACCOUNT_NAME as fallback). The old code read az://account/container/, which is inverted relative to how DataChain actually lists Azure — fixed.
  • bucket_overview passes the account to fsspec.filesystem("az", ...), strips the bare container (not the logical netloc) from adlfs entry names, and with --anon builds the chain on a session with client_config={"anon": True} so sampled File rows stay readable during enrichment. SKILL.md documents both.
  • bucket_scan needed no changes — the account rides through in the URI.

Guard: conflicting connection string

adlfs gives a connection string precedence over account_name, which would silently read account A while File.source says B. If the URI embeds an account and an explicit or env connection string names a different AccountName, the client now raises. A matching connection string works as before (this is also what the azurite functional test exercises). Documented in the remotes guide: account keys / SAS tokens / connection strings are account-specific, so mixing accounts in one run needs per-account-valid credentials (Azure AD or anonymous).

Not included (deliberately)

Closes #1859

🤖 Generated with Claude Code

@dreadatour
dreadatour requested review from Copilot and dmpetrov July 26, 2026 05:22
@dreadatour dreadatour self-assigned this Jul 26, 2026
@dreadatour
dreadatour marked this pull request as draft July 26, 2026 05:22
@dreadatour
dreadatour removed the request for review from dmpetrov July 26, 2026 05:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds support for embedding the Azure storage account in az:// URIs (az://container@account/...) so a single URI fully identifies both container and account, allowing multiple Azure accounts to coexist in a single run while preserving round-trippable dataset File.source values.

Changes:

  • Updated AzureClient to parse container@account, propagate account_name into filesystem kwargs, preserve @account when building clients from sources, and extend bucket_status to accept container@account.
  • Updated knowledge-skill scripts to build correct Azure HTTPS links and to pass/retain account information when producing bucket overviews (plus documentation updates).
  • Added/updated unit + functional tests to cover URI parsing, dataset-name sanitization, bucket status probing, and knowledge-skill behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/test_skill_knowledge_scripts.py Adds tests for Azure source_to_https behavior and bucket_overview Azure/account + anon-session wiring.
tests/unit/test_listing.py Ensures listing URI sanitization round-trips @ for Azure netlocs (container@account).
tests/unit/test_client_bucket_status.py Adds coverage for Azure bucket_status with container@account and conflicting connection-string guard.
tests/unit/test_client_azure.py Adds coverage for Azure client name/container/uri semantics with @account, env/kwargs precedence, and from_source preservation.
tests/func/test_client.py Functional coverage for scandir using az://container@account derived from connection string.
src/datachain/skill/knowledge/SKILL.md Documents Azure bucket-root extraction with embedded account and --anon usage for fallback overview.
src/datachain/skill/knowledge/scripts/utils.py Fixes Azure source_to_https to interpret az://container@account/... and supports env fallback.
src/datachain/skill/knowledge/scripts/bucket_overview.py Passes Azure account to fsspec.filesystem, strips bare container from adlfs entry names, and creates anon session for saved File readability.
src/datachain/client/azure.py Parses container@account at the client boundary, preserves @account in from_source, updates probes to use bare container, and adds connection-string mismatch guard.
src/datachain/client/init.py Updates bucket_status docstring to describe embedding Azure account in the URI as an alternative to account_name.
docs/guide/remotes.md Documents az://container@account/... and precedence/credential implications, including mismatch guard behavior.

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

Comment thread src/datachain/skill/knowledge/scripts/bucket_overview.py
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 26, 2026

Copy link
Copy Markdown

Deploying datachain with  Cloudflare Pages  Cloudflare Pages

Latest commit: fded3b3
Status: ✅  Deploy successful!
Preview URL: https://2ce7dda5.datachain-2g6.pages.dev
Branch Preview URL: https://az-account-name-2.datachain-2g6.pages.dev

View logs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

@dreadatour
dreadatour marked this pull request as ready for review July 26, 2026 16:20
@dreadatour
dreadatour requested review from a team, dmpetrov and shcheklein July 26, 2026 16:20
Comment thread tests/unit/test_skill_knowledge_scripts.py Outdated

@shcheklein shcheklein left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Do we still support regular fsspec URIs? do we support / respect Azure env variables?

(I think we don't want to create completely custom way of accessing things)

  • Can we check that @ in Azure is not reserved in some way to pass user name of some sort in some cases (ask to do full research on Azure URLs, custom deployments, etc, etc)

  • Ask to find all discussion on account name on fsspec, rclone forums - to double check on some stuff we are missing here potentially

  • How is it now compatible with older datasets where File doesn't have account name in them?

@amritghimire amritghimire left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests seems to be failing cc. @dreadatour

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support account in az:// URIs

4 participants