PyArrow: Keep the storage account out of ADLS paths in parse_location - #3884
PyArrow: Keep the storage account out of ADLS paths in parse_location#3884krishnakaanchan-png wants to merge 1 commit into
parse_location#3884Conversation
Fokko
left a comment
There was a problem hiding this comment.
Thanks @krishnakaanchan-png for fixing this, as far as my Azure knowledge goes, this looks good to me 👍
Head branch was pushed to by a user without write access
ae494a7 to
4c057f2
Compare
|
The three required checks here were never reporting. I have rebased on main and pushed. All three report now, and
That is an apt failure in the container build, nothing from the test suite ran. A re-run should clear it. Could someone re-run |
For Azure the netloc is <container>@<account>.<host>, so building the path as netloc + path put the account inside the path and PyArrow then read the whole first segment as the container name. Return only the container instead, which matches what PyArrow's own from_uri produces for the same location.
4c057f2 to
787130b
Compare
rambleraptor
left a comment
There was a problem hiding this comment.
This looks great. Thanks for doing this!
@Fokko could you put this in the merge queue?
Rationale for this change
parse_locationbuilds the path asf"{uri.netloc}{uri.path}". That is correct for S3, where the netloc is the bucket. For Azure the netloc iscontainer@account.dfs.core.windows.net, so the account ends up inside the path and PyArrow reads that whole first segment as the container name. EveryPyArrowFileIOcall on a canonical ABFS location is therefore pointed at a container that cannot exist, since container names allow only lowercase letters, numbers and hyphens.PyArrow itself handles these locations correctly.
FileSystem.from_uri("abfss://myfs@myacct.dfs.core.windows.net/wh/d.parquet")returnsmyfs/wh/d.parquet, so the problem is only on our side. After this changeparse_locationreturns the same path that PyArrow does.The full trace and the Azure documentation references are in #2698.
This is the path half only. The other half is that
_initialize_azure_fstakes no netloc, so the account can come only fromadls.account-nameand never from the location itself. I will send that separately, it needs a call on precedence when the property and the location disagree.Related to #2698. Not closing it here, since the account derivation is still pending.
Are these changes tested?
Yes. Two new parametrised tests over
abfs,abfss,wasbandwasbs, one for the account qualified form and one for the container only form.I also added an S3 case to
test_parse_location. That function has to keep serving both shapes and there was nothing pinning the S3 behaviour that the new branch has to preserve.make lintis clean andtests/io/test_pyarrow.pypasses.Are there any user-facing changes?
Yes. Locations of the form
abfs[s]://<container>@<account>.dfs.core.windows.net/<path>now resolve to the correct container underPyArrowFileIO. Earlier they resolved to a container named after the entire netloc. S3, HDFS and the container only Azure form are unchanged.