GH-41095: [C++][FS][Azure] Add support for CopyFile with hierarchical namespace support #41276
Conversation
…chical namespace support We need to add SAS (Shared Access Signatures) token for source URL.
|
|
cpp/src/arrow/filesystem/azurefs.cc
Outdated
| if (storage_shared_key_credential_) { | ||
| return builder->GenerateSasToken(*storage_shared_key_credential_); | ||
| } else { | ||
| // This part isn't tested. This may not work. |
There was a problem hiding this comment.
@Tom-Newton It seems that we can't use GetUserDelegationKey() with account key credential. We may need to use other authentication to test GetUserDelegationKey(). Do you know how to configure other authentication...?
(I think that we can defer this case to a separated issue.)
There was a problem hiding this comment.
I'm pretty sure its possible to create a SAS token from an account key but it probably isn't called delegated anymore.
There was a problem hiding this comment.
Yes. We can create a SAS token from an account key. The above builder->GenerateSasToken(*storage_shared_key_credential_) does it.
There was a problem hiding this comment.
Sorry I misunderstood the intention (I haven't found time to review this).
Configuring other auths in testing might be a bit difficult. For Azurite I think the first challenge is configuring https, which I have struggled with before. Beyond that I think there are 2 viable options offered by Azure:
- Service principal
- Username and password for a service account
- Surprisingly complicated to create
- Azure CLI
- Authenticates as your user
- Requires installing the Azure CLI and running
az login, then completing the login in your browser.
There was a problem hiding this comment.
Thanks.
I tried az login and ConfigureDefaultCredential() but some operations such as CreateDir() was blocked...
It seems that we need to implement GH-39344 to test this part.
I'll open a new issue for this part and defer this part in this PR.
There was a problem hiding this comment.
I tried az login and ConfigureDefaultCredential() but some operations such as CreateDir() was blocked...
It seems that we need to implement #39344 to test this part.
This seems a bit suspicious. Default credential does include CLI auth...
There was a problem hiding this comment.
Let's look at this in #41315.
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
github-actions
bot
added
awaiting merge
github-actions
bot
added
awaiting change review
|
I'll merge this. |
|
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 25bb627. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them. |
…chical namespace support (apache#41276) ### Rationale for this change We need to add SAS (Shared Access Signatures) token for source URL. ### What changes are included in this PR? Generate SAS token for source URL. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: apache#41095 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
…chical namespace support (apache#41276) ### Rationale for this change We need to add SAS (Shared Access Signatures) token for source URL. ### What changes are included in this PR? Generate SAS token for source URL. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: apache#41095 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
…chical namespace support (apache#41276) ### Rationale for this change We need to add SAS (Shared Access Signatures) token for source URL. ### What changes are included in this PR? Generate SAS token for source URL. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: apache#41095 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
…chical namespace support (apache#41276) ### Rationale for this change We need to add SAS (Shared Access Signatures) token for source URL. ### What changes are included in this PR? Generate SAS token for source URL. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: apache#41095 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
…chical namespace support (apache#41276) ### Rationale for this change We need to add SAS (Shared Access Signatures) token for source URL. ### What changes are included in this PR? Generate SAS token for source URL. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: apache#41095 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Rationale for this change
We need to add SAS (Shared Access Signatures) token for source URL.
What changes are included in this PR?
Generate SAS token for source URL.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.