You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Chinmay1220 (via GitHub)" <gi...@apache.org> on 2026/06/10 14:41:33 UTC
[PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Chinmay1220 opened a new pull request, #68344:
URL: https://github.com/apache/airflow/pull/68344
Fixes #40410.
This updates the ADLS ObjectStoragePath filesystem adapter to pass `account_name` to `adlfs.AzureBlobFileSystem` instead of an `account_url` keyword. `adlfs` requires `account_name`, and the previous logic could derive the account from `login`, which breaks ADLS V2 service principal connections where `login` is the client ID and `host` is the storage account name.
Changes:
- infer account name from `host` for `adls` connections while preserving the existing login fallback for legacy `wasb`-style connections
- add unit coverage for ADLS V2 service principal connections with plain account-name and DNS host values
- document ADLS ObjectStoragePath URL formats and connection fields
Tests:
- `python -m ruff check providers\microsoft\azure\src\airflow\providers\microsoft\azure\fs\adls.py providers\microsoft\azure\tests\unit\microsoft\azure\fs\test_adls.py`
- `python -m ruff format --check providers\microsoft\azure\src\airflow\providers\microsoft\azure\fs\adls.py providers\microsoft\azure\tests\unit\microsoft\azure\fs\test_adls.py`
- `python -m py_compile providers\microsoft\azure\src\airflow\providers\microsoft\azure\fs\adls.py providers\microsoft\azure\tests\unit\microsoft\azure\fs\test_adls.py`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "xlsongc (via GitHub)" <gi...@apache.org>.
xlsongc commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-6526975220
@Chinmay1220 aaron-y-chen approved after the live check, so what's left is potiuk's comment from 29 August on the custom-domain case in `test_adls.py`: it asserts `account_url`, which `AzureBlobFileSystem` drops, so it should either go or become an `account_host` case. The full DFS endpoint case just above it (`testaccountname.dfs.core.windows.net`) asserts the same key, so it likely needs the same change. The branch is also based on main from early August, so a rebase would get CI running again.
Do you still plan to finish this? If you're short on time, I'm happy to pick up these changes in a follow-up PR that keeps your commits.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub)" <gi...@apache.org>.
Chinmay1220 commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-4688143738
Thanks @dabla, good catch. I updated the PR to keep the previous `account_url` behavior for existing `wasb`, full URL, and custom endpoint connections.
The ADLS-specific change is now limited to the case that caused the parsing issue: `conn_type == "adls"` with a bare storage account name in `host`, where `login` is a client ID for Active Directory auth. In that case we pass `account_name` so the client ID is not incorrectly used to build the storage account URL.
I also updated the regression coverage to include:
- existing `wasb` connections still passing `account_url`
- ADLS bare account-name hosts passing `account_name`
- ADLS full DFS endpoint hosts still passing `account_url`
- ADLS custom endpoint hosts still passing `account_url`
Could you please take another look when you have bandwidth?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "dabla (via GitHub)" <gi...@apache.org>.
dabla commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-4688096537
Thanks for the PR.
I think this may introduce a breaking change for existing connections. Previously we passed `account_url` to `AzureBlobFileSystem` using:
```python
options = {
"account_url": parse_blob_account_url(conn.host, conn.login),
}
```
but this has now been replaced with deriving and passing `account_name` instead:
```python
account_name = _get_default_account_name(conn_type, conn.host, conn.login)
if account_name:
options["account_name"] = account_name
```
This means existing connections that rely on `account_url` semantics (for example custom endpoints) may no longer work after upgrading the provider.
Provider connection changes should always remain backward compatible, and existing connections must not be impacted by an upgrade. Am I missing something, or should we continue supporting the previous `account_url` behavior for backward compatibility?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub)" <gi...@apache.org>.
Chinmay1220 commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-4903951928
Friendly follow-up — this is rebased onto current `main` and CI is now fully green. I've also just pushed a `.bugfix.rst` newsfragment (`68344.bugfix.rst`).
Recap of the earlier backward-compatibility point @dabla raised: the change is scoped so that only `conn_type == "adls"` connections with a bare account-name host derive `account_name` (where `login` is the AD `client_id`). WASB, full-URL, and custom-endpoint connections still receive `account_url` via `parse_blob_account_url`, and regression tests cover all four cases — so existing connections are unaffected.
@dabla @potiuk would appreciate a review when you have the bandwidth. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub) via commits" <co...@airflow.apache.org>.
Chinmay1220 commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-6587538486
Thanks @xlsongc, both for the live check against a real ADLS Gen2 account and for the nudge. Yes, I'm still finishing this one, my bad for the slow turnaround.
Just pushed an update, rebased onto current `main`:
- **Custom-domain case** (@potiuk's 29 Aug inline): now an `account_host` case. The bare account name goes in `host` and the custom domain in `extra.account_host`, so the test asserts `account_name` + `account_host` (both real adlfs parameters) instead of the dropped `account_url`.
- **Full DFS endpoint case:** removed from the `test_get_fs` parametrize, since the only thing it could assert there was the same dead `account_url` key. What it was actually guarding, that a full endpoint is never treated as a bare account name, is now pinned directly by a small `test_bare_account_name_from_host` (dotted host, scheme, port, path, empty and `None`).
`test_adls.py` passes locally (19 passed, Linux / Python 3.12). No changes to `adls.py`. As discussed, the pre-existing `account_url` path for WASB and full-URL hosts is left as it is on `main`; happy to look at that in a separate PR.
I've also added the Gen-AI disclosure to the description, which I'd missed earlier.
@potiuk this should cover the last open point from your review, could you take another look when you have a moment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub)" <gi...@apache.org>.
Chinmay1220 commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-4780953495
@dabla this is ready for another look when you have bandwidth. I rebased onto the latest `main`, and the branch is now mergeable with the ADLS change still scoped to preserving existing `account_url` compatibility while fixing bare `adls` account-name handling.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #68344:
URL: https://github.com/apache/airflow/pull/68344#discussion_r3887008965
##########
providers/microsoft/azure/tests/unit/microsoft/azure/fs/test_adls.py:
##########
@@ -141,6 +139,126 @@ def mocked_blob_file_system():
"account_key": "password",
},
),
+ (
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="testaccountname",
+ login="client_id",
+ password="client_secret",
+ extra={
+ "tenant_id": "tenant_id",
+ },
+ ),
+ {
+ "account_name": "testaccountname",
+ "client_id": "client_id",
+ "client_secret": "client_secret",
+ "tenant_id": "tenant_id",
+ },
+ ),
+ (
+ # A mixed-case bare host is still an account name; urlparse lowercases it.
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="MyAccount",
+ login="client_id",
+ password="client_secret",
+ extra={
+ "tenant_id": "tenant_id",
+ },
+ ),
+ {
+ "account_name": "myaccount",
+ "client_id": "client_id",
+ "client_secret": "client_secret",
+ "tenant_id": "tenant_id",
+ },
+ ),
+ (
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="testaccountname",
+ password="account_key",
+ extra={},
+ ),
+ {
+ "account_name": "testaccountname",
+ "account_key": "account_key",
+ },
+ ),
+ (
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="testaccountname.dfs.core.windows.net",
+ login="client_id",
+ password="client_secret",
+ extra={
+ "tenant_id": "tenant_id",
+ },
+ ),
+ {
+ "account_url": "https://testaccountname.dfs.core.windows.net",
+ "client_id": "client_id",
+ "client_secret": "client_secret",
+ "tenant_id": "tenant_id",
+ },
+ ),
+ (
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="https://custom.blob.core.mydomain.io",
+ login="client_id",
+ password="client_secret",
+ extra={
+ "tenant_id": "tenant_id",
+ },
+ ),
+ {
+ "account_url": "https://custom.blob.core.mydomain.io",
Review Comment:
This case asserts `account_url` is produced, but adlfs never reads it — `AzureBlobFileSystem.__init__` has no such parameter, so it lands in `**kwargs` and is discarded, and adlfs computes its own `self.account_url` from `account_host`/`account_name` (giving `https://None.blob.core.windows.net` when `account_name` is unset).
So this case passes while pinning a runtime no-op. Either drop it, or make it an `account_host` case — that field is a real adlfs parameter, is already carried through the extras overlay, and is what the new docs point users at.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "aaron-y-chen (via GitHub)" <gi...@apache.org>.
aaron-y-chen commented on code in PR #68344:
URL: https://github.com/apache/airflow/pull/68344#discussion_r3679911354
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/fs/adls.py:
##########
@@ -47,12 +67,17 @@ def get_fs(conn_id: str | None, storage_options: dict[str, Any] | None = None) -
if connection_string:
return AzureBlobFileSystem(connection_string=connection_string)
- options: dict[str, Any] = {
- "account_url": parse_blob_account_url(conn.host, conn.login),
- }
-
# mirror handling of custom field "client_secret_auth_config" from extras. Ignore if missing as AzureBlobFileSystem can handle.
tenant_id = get_field(conn_id=conn_id, conn_type=conn_type, extras=extras, field_name="tenant_id")
+ options: dict[str, Any] = {}
+ account_name = _storage_account_name_from_host(conn.host)
+ # Preserve account_url for existing WASB, full URL, and custom endpoint connections.
+ # ADLS Active Directory connections use login as client_id, so bare hosts are account names.
+ if conn_type == "adls" and account_name:
+ options["account_name"] = account_name
+ elif conn_type != "adls" or conn.host or not tenant_id:
+ options["account_url"] = parse_blob_account_url(conn.host, conn.login)
Review Comment:
It seems `AzureBlobFileSystem` has no `account_url` parameter, it just falls into `**kwargs` ([ref](https://github.com/fsspec/adlfs/blob/2023.10.0/adlfs/spec.py#L226-L252)). Since `adls.rst` now says a full account URL can go in host, I'm afraid that path raises an error from `do_connect()` unless `account_name` is also provided.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #68344:
URL: https://github.com/apache/airflow/pull/68344#discussion_r3678127009
##########
providers/microsoft/azure/newsfragments/68344.bugfix.rst:
##########
@@ -0,0 +1 @@
+Fix ADLS ``ObjectStoragePath`` connection parsing to pass ``account_name`` for Active Directory ``adls`` connections while preserving the existing ``account_url`` behavior for WASB, full-URL, and custom-endpoint connections
Review Comment:
Please remove this file — `newsfragments/` doesn't apply to providers.
Provider release managers regenerate the changelog from `git log`, and only `airflow-core/`, `chart/` and `dev/mypy/` have a towncrier config to consume newsfragments, so nothing will ever pick this up. (There's one stray `51944.bugfix.rst` in this same directory on `main` — that slipped through rather than establishing a convention; it shouldn't be there either.)
If you want a user-visible note for this fix, add it directly to `providers/microsoft/azure/docs/changelog.rst` instead.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/fs/adls.py:
##########
@@ -29,6 +30,25 @@
schemes = ["abfs", "abfss", "adl"]
+def _account_name_from_host(host: str | None) -> str | None:
Review Comment:
As far as I can see in this diff, `_account_name_from_host` has exactly one caller — `_storage_account_name_from_host` — and that caller only returns the value when `account_name == host`. That condition can never hold when `host` contains a dot, so the `hostname.split(".", 1)[0]` here can never produce a value that's actually used.
The two helpers collapse into one that answers the real question: "is this host a bare account name?" Something like a single function returning `hostname if hostname == host.lower() else None` would say the same thing with half the surface.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/fs/adls.py:
##########
@@ -47,12 +67,17 @@ def get_fs(conn_id: str | None, storage_options: dict[str, Any] | None = None) -
if connection_string:
return AzureBlobFileSystem(connection_string=connection_string)
- options: dict[str, Any] = {
- "account_url": parse_blob_account_url(conn.host, conn.login),
- }
-
# mirror handling of custom field "client_secret_auth_config" from extras. Ignore if missing as AzureBlobFileSystem can handle.
tenant_id = get_field(conn_id=conn_id, conn_type=conn_type, extras=extras, field_name="tenant_id")
+ options: dict[str, Any] = {}
+ account_name = _storage_account_name_from_host(conn.host)
+ # Preserve account_url for existing WASB, full URL, and custom endpoint connections.
+ # ADLS Active Directory connections use login as client_id, so bare hosts are account names.
+ if conn_type == "adls" and account_name:
+ options["account_name"] = account_name
+ elif conn_type != "adls" or conn.host or not tenant_id:
Review Comment:
`elif conn_type != "adls" or conn.host or not tenant_id:` packs three independent reasons into one condition, and the case it deliberately leaves out — `adls` + no host + a tenant_id, where *neither* `account_name` nor `account_url` is set so adlfs falls back to `DefaultAzureCredential` — is invisible unless you work the truth table by hand.
Worth either splitting into explicit branches with a short comment on the fall-through case, or adding a test that pins it, since it's the one combination with no key set at all.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/fs/adls.py:
##########
@@ -29,6 +30,25 @@
schemes = ["abfs", "abfss", "adl"]
+def _account_name_from_host(host: str | None) -> str | None:
+ if not host:
+ return None
+
+ parsed_url = urlparse(host if "://" in host else f"//{host}")
+ hostname = parsed_url.hostname
+ if not hostname:
+ return None
+
+ return hostname.split(".", 1)[0]
+
+
+def _storage_account_name_from_host(host: str | None) -> str | None:
+ account_name = _account_name_from_host(host)
+ if account_name and account_name == host:
Review Comment:
`urlparse(...).hostname` lowercases, so this equality is case-sensitive in a surprising direction:
```python
>>> urlparse("//MyAccount").hostname
'myaccount'
```
With `host="MyAccount"` the comparison is `"myaccount" == "MyAccount"` -> `False`, so the connection silently falls through to the `account_url` branch instead of `account_name` — i.e. the bug this PR fixes still reproduces purely because of host casing. Azure storage account names are lowercase by spec, so it's an edge, but it's the kind that produces a confusing report later.
Comparing `account_name == host.lower()` (or normalising `host` once up front) would make it case-insensitive.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub)" <gi...@apache.org>.
Chinmay1220 commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-5444027439
Thanks @aaron-y-chen, that's fair. I'll set up an ADLS Gen2 account with a service principal and verify a bare-host `adls` connection reads and writes end to end against real Azure, then post the results here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub) via commits" <co...@airflow.apache.org>.
Chinmay1220 commented on code in PR #68344:
URL: https://github.com/apache/airflow/pull/68344#discussion_r4512381675
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/fs/adls.py:
##########
@@ -47,12 +67,17 @@ def get_fs(conn_id: str | None, storage_options: dict[str, Any] | None = None) -
if connection_string:
return AzureBlobFileSystem(connection_string=connection_string)
- options: dict[str, Any] = {
- "account_url": parse_blob_account_url(conn.host, conn.login),
- }
-
# mirror handling of custom field "client_secret_auth_config" from extras. Ignore if missing as AzureBlobFileSystem can handle.
tenant_id = get_field(conn_id=conn_id, conn_type=conn_type, extras=extras, field_name="tenant_id")
+ options: dict[str, Any] = {}
+ account_name = _storage_account_name_from_host(conn.host)
+ # Preserve account_url for existing WASB, full URL, and custom endpoint connections.
+ # ADLS Active Directory connections use login as client_id, so bare hosts are account names.
+ if conn_type == "adls" and account_name:
+ options["account_name"] = account_name
+ elif conn_type != "adls" or conn.host or not tenant_id:
+ options["account_url"] = parse_blob_account_url(conn.host, conn.login)
Review Comment:
Addressed in 0282cb2: `adls.rst` now says Host is the bare storage account name, and points private endpoints and custom domains at the `account_host` field instead of a full URL. So the docs no longer lead to the dropped `account_url` path.
##########
providers/microsoft/azure/tests/unit/microsoft/azure/fs/test_adls.py:
##########
@@ -141,6 +139,126 @@ def mocked_blob_file_system():
"account_key": "password",
},
),
+ (
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="testaccountname",
+ login="client_id",
+ password="client_secret",
+ extra={
+ "tenant_id": "tenant_id",
+ },
+ ),
+ {
+ "account_name": "testaccountname",
+ "client_id": "client_id",
+ "client_secret": "client_secret",
+ "tenant_id": "tenant_id",
+ },
+ ),
+ (
+ # A mixed-case bare host is still an account name; urlparse lowercases it.
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="MyAccount",
+ login="client_id",
+ password="client_secret",
+ extra={
+ "tenant_id": "tenant_id",
+ },
+ ),
+ {
+ "account_name": "myaccount",
+ "client_id": "client_id",
+ "client_secret": "client_secret",
+ "tenant_id": "tenant_id",
+ },
+ ),
+ (
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="testaccountname",
+ password="account_key",
+ extra={},
+ ),
+ {
+ "account_name": "testaccountname",
+ "account_key": "account_key",
+ },
+ ),
+ (
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="testaccountname.dfs.core.windows.net",
+ login="client_id",
+ password="client_secret",
+ extra={
+ "tenant_id": "tenant_id",
+ },
+ ),
+ {
+ "account_url": "https://testaccountname.dfs.core.windows.net",
+ "client_id": "client_id",
+ "client_secret": "client_secret",
+ "tenant_id": "tenant_id",
+ },
+ ),
+ (
+ Connection(
+ conn_id="testconn",
+ conn_type="adls",
+ host="https://custom.blob.core.mydomain.io",
+ login="client_id",
+ password="client_secret",
+ extra={
+ "tenant_id": "tenant_id",
+ },
+ ),
+ {
+ "account_url": "https://custom.blob.core.mydomain.io",
Review Comment:
Done in 5e829e2: this is now an `account_host` case (bare account name in `host`, custom domain in `extra.account_host`) that asserts `account_name` + `account_host`. The full DFS endpoint case above it had the same problem, so 0cdc7cf replaces it with a direct test of `_bare_account_name_from_host`. Happy to change it if you'd prefer another approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub)" <gi...@apache.org>.
Chinmay1220 commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-4780918961
Rebased this branch on the latest `main` and pushed the refreshed head (`a9a6a0ffb5`). The previous red checks were from the stale base; the ADLS change is still scoped to bare `adls` account-name hosts and preserves existing `account_url` behavior for WASB, full URL, and custom endpoint connections.
This is ready for review/workflow approval when you have bandwidth.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub)" <gi...@apache.org>.
Chinmay1220 commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-4686117344
Hi @dabla and @bolkedebruin, could you please review this PR when you have bandwidth?
It fixes ADLS `ObjectStoragePath` account-name parsing for Azure object storage paths and adds focused regression coverage. The PR is scoped to the Microsoft Azure provider path parsing behavior.
Happy to adjust anything you'd like changed. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix ADLS ObjectStoragePath connection parsing [airflow]
Posted by "Chinmay1220 (via GitHub)" <gi...@apache.org>.
Chinmay1220 commented on PR #68344:
URL: https://github.com/apache/airflow/pull/68344#issuecomment-5209540851
Thanks for the thorough review @potiuk, and @aaron-y-chen for digging into the adlfs internals. Just pushed an update (rebased onto latest `main`) that addresses everything:
- **Case-sensitivity + dead helper:** collapsed the two functions into a single `_bare_account_name_from_host` that compares against `host.lower()`, so a mixed-case bare host like `MyAccount` now resolves to `account_name` instead of silently falling through to `account_url`. Kept a `"." not in hostname` guard so full endpoints (`acct.dfs.core.windows.net`) still take the `account_url` path. Added a test for the mixed-case host.
- **Packed condition:** left the fall-through case (`adls` + no host + `tenant_id`) as an explicit comment noting it intentionally sets neither key and lets adlfs use `DefaultAzureCredential`. The existing parametrized test already pins that combination.
- **Newsfragment:** removed — didn't realize providers regenerate the changelog from `git log`. Thanks for the pointer.
- **Doc / `account_url`:** you're right, @aaron-y-chen. `AzureBlobFileSystem` never reads a passed `account_url` — `do_connect` builds its own from `account_name` (or `account_host`) and raises if neither that nor a connection string is set. So a full URL in `host` wouldn't actually connect. I've reworded the filesystem doc to point private endpoints / custom domains at the `account_host` field instead.
Happy to split the fall-through into explicit branches instead of a comment if you'd prefer that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org