You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ephraimbuddy (via GitHub)" <gi...@apache.org> on 2023/08/16 21:08:33 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request, #33457: Fix updating account url for WasbHook
ephraimbuddy opened a new pull request, #33457:
URL: https://github.com/apache/airflow/pull/33457
There are different ways users supply the hostname(account url) in Azure, sometimes the host doesn't have a urlparse.scheme but has urlparse.path e.g. name.blob.windows.net and other times, it will just be Azure ID e.g. aldhjf9dads. While working on #32980, I assumed that if there's no scheme, then the hostname is not valid, that's incorrect since DNS can serve as the host.
The fix was to check if we don't have netloc and that urlparse.path does not include a dot and if it does not, use the login/account_name to construct the account_url
Closes: https://github.com/apache/airflow/issues/33203
--
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
[GitHub] [airflow] ephraimbuddy merged pull request #33457: Fix updating account url for WasbHook
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy merged PR #33457:
URL: https://github.com/apache/airflow/pull/33457
--
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
[GitHub] [airflow] hussein-awala commented on a diff in pull request #33457: Fix updating account url for WasbHook
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #33457:
URL: https://github.com/apache/airflow/pull/33457#discussion_r1296481594
##########
airflow/providers/microsoft/azure/hooks/wasb.py:
##########
@@ -152,11 +153,13 @@ def get_conn(self) -> BlobServiceClient:
# connection_string auth takes priority
return BlobServiceClient.from_connection_string(connection_string, **extra)
- account_url = (
- conn.host
- if conn.host and conn.host.startswith("https://")
- else f"https://{conn.login}.blob.core.windows.net/"
- )
+ account_url = conn.host if conn.host else f"https://{conn.login}.blob.core.windows.net/"
+ parsed_url = urlparse(account_url)
+
+ if not parsed_url.netloc and "." not in parsed_url.path:
+ # if there's no netloc and no dots in the path, then user only
+ # provided the host ID, not the full URL or DNS name
+ account_url = f"https://{conn.login}.blob.core.windows.net/"
Review Comment:
What do you mean by `host ID`? Is it account name? I wonder if we really need to skip the provided host or we should do something like:
```python
account_url = f"https://{conn.host}.blob.core.windows.net/"
```
--
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
[GitHub] [airflow] ephraimbuddy commented on a diff in pull request #33457: Fix updating account url for WasbHook
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #33457:
URL: https://github.com/apache/airflow/pull/33457#discussion_r1296528091
##########
airflow/providers/microsoft/azure/hooks/wasb.py:
##########
@@ -152,11 +153,13 @@ def get_conn(self) -> BlobServiceClient:
# connection_string auth takes priority
return BlobServiceClient.from_connection_string(connection_string, **extra)
- account_url = (
- conn.host
- if conn.host and conn.host.startswith("https://")
- else f"https://{conn.login}.blob.core.windows.net/"
- )
+ account_url = conn.host if conn.host else f"https://{conn.login}.blob.core.windows.net/"
+ parsed_url = urlparse(account_url)
+
+ if not parsed_url.netloc and "." not in parsed_url.path:
+ # if there's no netloc and no dots in the path, then user only
+ # provided the host ID, not the full URL or DNS name
+ account_url = f"https://{conn.login}.blob.core.windows.net/"
Review Comment:
Not the host ID, sorry, I'll update that. What I mean is the Active Directory ID.
The host field is named Active Directory Auth so at times people use the active directory ID there and it seems to be there even before we upgraded to blob storage v12
We can’t create the url with the Active Directory ID. We can only create it with the account name/login. That’s what works
##########
airflow/providers/microsoft/azure/hooks/wasb.py:
##########
@@ -152,11 +153,13 @@ def get_conn(self) -> BlobServiceClient:
# connection_string auth takes priority
return BlobServiceClient.from_connection_string(connection_string, **extra)
- account_url = (
- conn.host
- if conn.host and conn.host.startswith("https://")
- else f"https://{conn.login}.blob.core.windows.net/"
- )
+ account_url = conn.host if conn.host else f"https://{conn.login}.blob.core.windows.net/"
+ parsed_url = urlparse(account_url)
+
+ if not parsed_url.netloc and "." not in parsed_url.path:
+ # if there's no netloc and no dots in the path, then user only
+ # provided the host ID, not the full URL or DNS name
+ account_url = f"https://{conn.login}.blob.core.windows.net/"
Review Comment:
Not the host ID, sorry, I'll update that. What I mean is the Active Directory ID.
The host field is named Active Directory Auth so at times people use the active directory ID there and it seems to be there even before we upgraded to blob storage v12
We can’t create the url with the Active Directory ID. We can only create it with the account name/login. That’s what works
--
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