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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to