You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "tom-s-powell (via GitHub)" <gi...@apache.org> on 2025/09/11 13:01:25 UTC
[PR] Fix prefix match for ADLS_SAS_TOKEN [iceberg-python]
tom-s-powell opened a new pull request, #2461:
URL: https://github.com/apache/iceberg-python/pull/2461
The current logic when constructing an ADLS file system incorrectly matches `adls.sas-token-expires-at-ms.` prefixed keys. If this is provided and encountered prior to `adls.sas-token.` prefixed keys, it will incorrectly determine the `ADLS_ACCOUNT_NAME` and `ADLS_SAS_TOKEN`.
For example with the following properties:
```
{
'adls.sas-token-expires-at-ms.testaccount.dfs.core.windows.net': '1757597218121',
'adls.sas-token.testaccount.dfs.core.windows.net': '<redacted>'
}
```
**Before**
```
{
'adls.sas-token-expires-at-ms.testaccount.dfs.core.windows.net': '1757597218121',
'adls.sas-token.testaccount.dfs.core.windows.net': '<redacted>',
'adls.account-name': 'adls',
'adls.sas-token': '1757597218121'
}
```
**After**
```
{
'adls.sas-token-expires-at-ms.testaccount.dfs.core.windows.net': '1757597218121',
'adls.sas-token.testaccount.dfs.core.windows.net': '<redacted>',
'adls.account-name': 'testaccount',
'adls.sas-token': '<redacted>'
}
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix prefix match for ADLS_SAS_TOKEN [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #2461:
URL: https://github.com/apache/iceberg-python/pull/2461#issuecomment-3300059657
Thanks for the PR @tom-s-powell and @smaheshwar-pltr @Fokko for the review :)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix prefix match for ADLS_SAS_TOKEN [iceberg-python]
Posted by "smaheshwar-pltr (via GitHub)" <gi...@apache.org>.
smaheshwar-pltr commented on PR #2461:
URL: https://github.com/apache/iceberg-python/pull/2461#issuecomment-3281256735
@kevinjqliu @Fokko, please may you take a look, so the client still works if Catalogs set the refresh property?
(I see that the 1.10 vote just passed so suppose this'd have to be for the next release 😄)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix prefix match for ADLS_SAS_TOKEN [iceberg-python]
Posted by "smaheshwar-pltr (via GitHub)" <gi...@apache.org>.
smaheshwar-pltr commented on code in PR #2461:
URL: https://github.com/apache/iceberg-python/pull/2461#discussion_r2341291558
##########
pyiceberg/io/fsspec.py:
##########
@@ -207,7 +207,7 @@ def _adls(properties: Properties) -> AbstractFileSystem:
from azure.core.credentials_async import AsyncTokenCredential
for key, sas_token in {
- key.replace(f"{ADLS_SAS_TOKEN}.", ""): value for key, value in properties.items() if key.startswith(ADLS_SAS_TOKEN)
+ key.replace(f"{ADLS_SAS_TOKEN}.", ""): value for key, value in properties.items() if key.startswith(f"{ADLS_SAS_TOKEN}.")
Review Comment:
This looks like the fix for the bug 👍, the Java side gets the props like [this](https://github.com/apache/iceberg/blob/d1771207c9040f1c8b6886665b56d1a972fe402a/azure/src/main/java/org/apache/iceberg/azure/AzureProperties.java#L76) with the `ADLS_SAS_TOKEN_PREFIX` including the `.`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix prefix match for ADLS_SAS_TOKEN [iceberg-python]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on PR #2461:
URL: https://github.com/apache/iceberg-python/pull/2461#issuecomment-3285415828
Added a test @Fokko if this gets at what you were thinking?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix prefix match for ADLS_SAS_TOKEN [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #2461:
URL: https://github.com/apache/iceberg-python/pull/2461#issuecomment-3288475612
@tom-s-powell Yes, that looks great. 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix prefix match for ADLS_SAS_TOKEN [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #2461:
URL: https://github.com/apache/iceberg-python/pull/2461
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix prefix match for ADLS_SAS_TOKEN [iceberg-python]
Posted by "smaheshwar-pltr (via GitHub)" <gi...@apache.org>.
smaheshwar-pltr commented on code in PR #2461:
URL: https://github.com/apache/iceberg-python/pull/2461#discussion_r2341291558
##########
pyiceberg/io/fsspec.py:
##########
@@ -207,7 +207,7 @@ def _adls(properties: Properties) -> AbstractFileSystem:
from azure.core.credentials_async import AsyncTokenCredential
for key, sas_token in {
- key.replace(f"{ADLS_SAS_TOKEN}.", ""): value for key, value in properties.items() if key.startswith(ADLS_SAS_TOKEN)
+ key.replace(f"{ADLS_SAS_TOKEN}.", ""): value for key, value in properties.items() if key.startswith(f"{ADLS_SAS_TOKEN}.")
Review Comment:
This looks like the fix for the bug 👍, the Java side gets the props like [this](https://github.com/apache/iceberg/blob/d1771207c9040f1c8b6886665b56d1a972fe402a/azure/src/main/java/org/apache/iceberg/azure/AzureProperties.java#L76) with the `ADLS_SAS_TOKEN_PREFIX` _including_ the `.`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org