pankajastro commented on code in PR #73374:
URL: https://github.com/apache/airflow/pull/73374#discussion_r4147141948


##########
providers/common/sql/src/airflow/providers/common/sql/datafusion/engine.py:
##########
@@ -213,6 +317,50 @@ def _remove_none_values(params: dict[str, Any]) -> 
dict[str, Any]:
         """Filter out None values from the dictionary."""
         return {k: v for k, v in params.items() if v is not None}
 
+    _AZURE_PUBLIC_SUFFIX = ".blob.core.windows.net"
+
+    @classmethod
+    def _resolve_wasb_account(cls, host: str | None, login: str | None) -> str 
| None:
+        """
+        Return the storage account name the way WasbHook resolves it.
+
+        From ``host`` when set (its netloc's first label), falling back to 
``login`` only when
+        ``host`` is empty -- login holds the service-principal client_id in 
that auth mode, not
+        the account name. Returns ``None`` when neither is set, so the binding 
falls back to
+        ``AZURE_STORAGE_ACCOUNT_NAME`` instead of targeting the literal string 
``"None"``.
+        Reimplemented locally rather than importing
+        ``airflow.providers.microsoft.azure.utils.parse_blob_account_url``, to 
avoid pulling the
+        microsoft-azure provider's full Azure SDK dependency stack into 
common-sql for one string
+        operation that only needs the stdlib.
+
+        Only the public ``*.blob.core.windows.net`` cloud is supported unless 
the worker sets
+        ``AZURE_STORAGE_ENDPOINT``/``AZURE_ENDPOINT``: DataFusion's Azure 
binding takes no
+        endpoint override on this side, so a sovereign-cloud or emulator host 
would otherwise be
+        silently misrouted to the public account of the same name -- but once 
one of those
+        variables is set, the binding uses it verbatim instead of deriving the 
URL from the
+        account name, so the host no longer needs to match.
+        """
+        if not host and not login:
+            return None
+        netloc = urlsplit(host if host else 
f"https://{login}.blob.core.windows.net/";).netloc
+        if not netloc:
+            # No scheme was given (e.g. a bare DNS name); urlsplit put it all 
in the path instead.
+            netloc = urlsplit(f"https://{host}";).netloc
+        if "." not in netloc:
+            # Only an Active Directory ID was given, not a full URL or DNS 
name.
+            netloc = f"{login}.blob.core.windows.net"
+        if not netloc.endswith(cls._AZURE_PUBLIC_SUFFIX) and not (
+            os.environ.get("AZURE_STORAGE_ENDPOINT") or 
os.environ.get("AZURE_ENDPOINT")
+        ):
+            raise ValueError(
+                f"Connection host {host!r} does not resolve to the public 
{cls._AZURE_PUBLIC_SUFFIX} "
+                "cloud, which is the only one DataFusion's Azure Blob Storage 
binding can target (it "
+                "has no endpoint override). Sovereign clouds and the Azurite 
emulator are not "
+                "supported unless the AZURE_STORAGE_ENDPOINT environment 
variable is set."

Review Comment:
   Fixed with the first option, and had to go further than the endpoint skip 
alone — an Azurite host without `login` (bare `azurite` or `azurite:10000`) was 
falling through the pre-existing Active-Directory-ID fallback and silently 
resolving to the literal string `"None"` instead of raising. Colon-shaped or 
dotless-without-`login` hosts now always raise, pointing at the 
`login`+empty-`host` workaround; a real sovereign-cloud hostname with the 
endpoint var set still works.
   
   ---
   Drafted-by: Claude Code (Sonnet 5); reviewed by @pankajastro before posting



##########
providers/common/sql/docs/operators.rst:
##########
@@ -362,6 +363,57 @@ resolved in this order:
     :start-after: [START howto_analytics_operator_with_gcs]
     :end-before: [END howto_analytics_operator_with_gcs]
 
+Azure Storage
+-------------
+Use an ``az://`` URI with a ``conn_id`` pointing to a ``wasb`` connection.
+``abfs://`` and ``abfss://`` URIs are not recognized yet. The account name
+comes from ``host`` (its first DNS label) when set, falling back to
+``login`` only when ``host`` is empty; only the public
+``*.blob.core.windows.net`` cloud is supported unless the worker sets
+``AZURE_STORAGE_ENDPOINT``/``AZURE_ENDPOINT``, since DataFusion's binding
+otherwise has no endpoint override. ``client_secret_auth_config`` (the
+authority override ``WasbHook`` honors) is not read here.
+
+The connection supplies one of the following credentials, checked in this
+order (matching ``WasbHook.get_conn``):
+
+1. Azure AD service principal -- ``tenant_id`` extra, with ``login`` as the
+   client ID and ``password`` as the client secret (both required together)
+2. Shared key -- the ``shared_access_key`` extra
+3. SAS token -- ``sas_token`` extra, as a query string
+4. Shared key -- ``password``, or the ``account_key`` extra
+5. None of the above -- ambient auth (see below)
+
+**A worker environment variable can override the connection.** DataFusion
+reads ``AZURE_*`` environment variables first, and an environment bearer
+token, access key, workload-identity token, or client secret wins over the
+connection's SAS token or shared key (a SAS token is the lowest-precedence

Review Comment:
   Fixed, thanks — docs now describe the per-tier rule directly instead of one 
flat claim.
   
   ---
   Drafted-by: Claude Code (Sonnet 5); reviewed by @pankajastro 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: [email protected]

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

Reply via email to