New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Azure blob storage sas token #51141
Azure blob storage sas token #51141
Conversation
This is an automated comment for commit 74e7ff9 with description of existing statuses. It's updated for the latest CI running
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SmitaRKulkarni All azure tests failed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test failure looks related
src/Storages/StorageAzureBlob.cpp
Outdated
|
||
if (!container_exists) | ||
{ | ||
result->CreateIfNotExists(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It it's readonly case maybe it make sense to throw exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to pass parameter is_insert_query
to table function (had to update all table functions, but this parameter is currently only used in azureBlobStrorage). If its not insert query and container does not exist, we throw error.
…reate it, added this flag for all table functions, only used in azureBlobStorage
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Updated check for connection_string as connection string with sas does not always begin with DefaultEndPoint and updated connection url to include sas token after adding container to url.
cc @alesapin