SNOW-3834837 migrate azure sdk to Azure SDK for C++ - #1044
SNOW-3834837 migrate azure sdk to Azure SDK for C++#1044sfc-gh-ext-simba-hx wants to merge 10 commits into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
9cad311 to
47b927a
Compare
47b927a to
96a13fa
Compare
|
|
||
| try { | ||
| Azure::Storage::Blobs::BlobClientOptions options; | ||
| options.Retry.MaxRetries = m_maxRetries; |
There was a problem hiding this comment.
m_maxRetries seems to be used uninitialized.
FileTransferAgent calls setMaxRetries() after getClient(), but Azure only reads retry options at construction. setMaxRetries only assigns the member and does not recreate/update the client.
There was a problem hiding this comment.
addressed by passing maxRetries to Azure client constructor.
| return m_length; | ||
| } | ||
|
|
||
| void Rewind() override |
There was a problem hiding this comment.
Single-part upload uses this adapter around a non-seekable cipher stream. If MaxRetries > 0, Azure retry can re-read from EOF and upload empty/corrupt data or fail opaquely.
There was a problem hiding this comment.
Created m_blobServiceClientNoRetry with MaxRetries set to 0 for single-part upload.
| @@ -79,31 +82,48 @@ SnowflakeAzureClient::SnowflakeAzureClient(StageInfo *stageInfo, | |||
| std::string account_name = m_stageInfo->storageAccount; | |||
| std::string sas_key = m_stageInfo->credentials[azuresaskey]; | |||
| std::string endpoint = account_name + "." + m_stageInfo->endPoint; | |||
There was a problem hiding this comment.
cpplite client prefixed a scheme automatically, but the new one does not. Let's make sure that when https is enabled, the scheme prefix is there.
There was a problem hiding this comment.
Existing code with cpplite hardcoded use_https as true. Hardcoded to add https prefix here.
| if (copylen > size) { | ||
| return SF_STATUS_ERROR_BUFFER_TOO_SMALL; | ||
| } | ||
| sf_strncpy(value, size, CLIENT_CONFIG_FILE, size); |
There was a problem hiding this comment.
| sf_strncpy(value, size, CLIENT_CONFIG_FILE, size); | |
| sf_strncpy(value, size, CLIENT_CONFIG_FILE, copylen); |
There was a problem hiding this comment.
addressed
| auto response = blobClient.Download(); | ||
| if (!response.Value.BodyStream) | ||
| { | ||
| CXX_LOG_ERROR("%s file donwload failed:: BodyStream is NULL", |
There was a problem hiding this comment.
| CXX_LOG_ERROR("%s file donwload failed:: BodyStream is NULL", | |
| CXX_LOG_ERROR("%s file download failed:: BodyStream is NULL", |
There was a problem hiding this comment.
addressed
| virtual bool IsExpired() = 0; | ||
|
|
||
| + /** | ||
| + * @brief Checks whether this CURL connection is ued proxy. |
There was a problem hiding this comment.
| + * @brief Checks whether this CURL connection is ued proxy. | |
| + * @brief Checks whether this CURL connection is used proxy. |
There was a problem hiding this comment.
addressed
2 participants
migrate to the latest azure sdk for c++ (azure-stoarge-blobs 12.18.0) from the deprecated azure-storage-cpplite (will remove in a separated PR to keep this PR in a reviewable amount of changed files)
Also fixed reading overflow around sf_strncpy which caused segment fault in test_unit_logger after the migration, tighten the boundary check and null terminator.