pankajastro commented on code in PR #73374:
URL: https://github.com/apache/airflow/pull/73374#discussion_r4136428687
##########
providers/common/sql/docs/operators.rst:
##########
@@ -362,6 +363,49 @@ 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, since DataFusion's binding
+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:
+
+1. Azure AD service principal -- ``tenant_id`` extra, with ``login`` as the
+ client ID and ``password`` as the client secret (both required together)
+2. SAS token -- ``sas_token`` extra, as a query string
+3. Shared key -- ``password``, or the ``shared_access_key``/``account_key``
extra
+4. 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 access
+key or workload-identity token wins over the connection's SAS token or
+client secret. If the connection sets an explicit credential (1-3 above)
+and the worker also has ``AZURE_FEDERATED_TOKEN_FILE``,
+``AZURE_STORAGE_ACCOUNT_KEY``, ``AZURE_STORAGE_ACCESS_KEY``,
+``AZURE_STORAGE_SAS_KEY``, or ``AZURE_STORAGE_TOKEN`` set, this raises
Review Comment:
Fixed, thanks — docs now list the same set as the code
(`AZURE_STORAGE_TOKEN`, `AZURE_STORAGE_ACCOUNT_KEY`,
`AZURE_STORAGE_ACCESS_KEY`, `AZURE_STORAGE_MASTER_KEY`,
`AZURE_FEDERATED_TOKEN_FILE`, or the client-secret triple).
---
Drafted-by: Claude Code (Sonnet 5); reviewed by @pankajastro before posting
##########
providers/common/sql/src/airflow/providers/common/sql/datafusion/engine.py:
##########
@@ -213,6 +294,45 @@ 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:
DataFusion's Azure binding
+ takes no endpoint override, so a sovereign-cloud or emulator host
would otherwise be
+ silently misrouted to the public account of the same name.
+ """
+ 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):
+ 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; set the AZURE_STORAGE_ENDPOINT environment
variable instead."
Review Comment:
Went with the first option — now skips the suffix check when
`AZURE_STORAGE_ENDPOINT` or `AZURE_ENDPOINT` is already set, so the advice this
message gives actually works.
---
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]
