Skip to content

Support az:// storage account name - #1798

Closed
dreadatour wants to merge 10 commits into
mainfrom
az-account-name
Closed

Support az:// storage account name#1798
dreadatour wants to merge 10 commits into
mainfrom
az-account-name

Conversation

@dreadatour

@dreadatour dreadatour commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

One of the Claude's findings along the way.

The storage account isn't part of an az:// URI (the netloc is the container), so file links and access probes must receive it explicitly. This threads the account name through the whole knowledge-skill scan workflow.

  • source_to_https(): take an explicit account_name, falling back to AZURE_STORAGE_ACCOUNT_NAME; build the blob URL from account + container, and return None for az:// when the account (or container) is missing rather than emitting a malformed link. The env fallback also gives dataset previews (schema.py) az links for free.
  • bucket_scan: add --account-name (defaulting to AZURE_STORAGE_ACCOUNT_NAME) and thread it as a generic client_config through the access probe and the read_storage listing (so adlfs can resolve the account). The probe now honors the script's error contract: transport failures (e.g. a stale env account name) emit the error JSON and exit 1 instead of a traceback, and a probe timeout exits 124.
  • bucket_overview (timeout fallback): same --account-name flag, threaded into the fsspec listing, the bucket JSON, and the chain's session (Session.get(client_config=...)) so sampled File content stays readable during enrichment.
  • SKILL.md: document --account-name for all three Step-1 commands (the datachain bucket status CLI has no env default) and --anon for the fallback when the access check reported anonymous.

🤖 Generated with Claude Code

@dreadatour
dreadatour requested review from a team, Copilot and dmpetrov June 4, 2026 12:26
@dreadatour dreadatour self-assigned this Jun 4, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 4, 2026

Copy link
Copy Markdown

Deploying datachain with  Cloudflare Pages  Cloudflare Pages

Latest commit: 57e0260
Status: ✅  Deploy successful!
Preview URL: https://cf50f345.datachain-2g6.pages.dev
Branch Preview URL: https://az-account-name.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

This PR updates the knowledge “bucket scan” tooling to correctly handle Azure Blob Storage URIs (az://) where the URI netloc is the container and the storage account name must be supplied separately to build HTTPS file links and to perform the anonymous-access probe.

Changes:

  • Extend source_to_https() to accept an explicit account_name and return None for az:// when it’s missing.
  • Add --account-name to bucket_scan.py, defaulting to AZURE_STORAGE_ACCOUNT_NAME, and thread it through the Azure anonymous-access probe, listing (client_config), and link prefix generation.
  • Update unit tests to cover Azure link generation with/without an explicit account name.

Reviewed changes

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

File Description
tests/unit/test_skill_knowledge_scripts.py Updates tests for Azure link prefix behavior with explicit account_name and None when missing.
src/datachain/skill/knowledge/scripts/utils.py Changes source_to_https() to require account_name for az:// HTTPS prefix construction.
src/datachain/skill/knowledge/scripts/bucket_scan.py Adds --account-name option and threads Azure account name into probing, listing configuration, and link prefix creation.

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

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

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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 3 out of 3 changed files in this pull request and generated no new comments.

Comment thread src/datachain/skill/knowledge/scripts/bucket_scan.py Outdated

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 5 out of 5 changed files in this pull request and generated no new comments.

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 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread src/datachain/skill/knowledge/scripts/bucket_scan.py
@dreadatour
dreadatour requested a review from Copilot July 14, 2026 19:39
@dreadatour

Copy link
Copy Markdown
Contributor Author

@dmpetrov I would like you to also take a look at these changes, what do you think?

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 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread tests/unit/test_skill_knowledge_scripts.py

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 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread src/datachain/skill/knowledge/scripts/bucket_overview.py
Comment thread src/datachain/skill/knowledge/scripts/utils.py

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 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread tests/unit/test_skill_knowledge_scripts.py
@dmpetrov

Copy link
Copy Markdown
Contributor

@dreadatour I've look breafly. How about an alternative approach #1859?

@dreadatour

Copy link
Copy Markdown
Contributor Author

@dreadatour I've look breafly. How about an alternative approach #1859?

Hm, sounds good to me! I haven't checked other products — my bad. Let me dive into the alternative approach 👍

@dreadatour

Copy link
Copy Markdown
Contributor Author

Closing in favor of #1887

@dreadatour dreadatour closed this Jul 26, 2026
@dreadatour
dreadatour deleted the az-account-name branch July 26, 2026 16:25
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.

5 participants