pitrou commented on PR #12914:
URL: https://github.com/apache/arrow/pull/12914#issuecomment-1210792751

   I'd like to give this a review before it gets merged, if possible.
   
   Le 10 août 2022 10:28:06 Sutou Kouhei ***@***.***> a écrit :
   >
   > @kou commented on this pull request.
   > Could you rebase on master to resolve conflicts?
   > Could you tell me your account on Jira? I want to assign you to 
   > https://issues.apache.org/jira/browse/ARROW-2034 before we merge this.
   > Honestly, I can't review all changes carefully in this pull request 
because 
   > this is too large for me. I'll merge this after we address all comments 
   > from me. Problems I missed in this pull request or newly found problems 
   > after we merge can be fixed by follow-up pull requests.
   >
   > In cpp/thirdparty/versions.txt:
   >> +ARROW_AZURE_STORAGE_BLOB_BUILD_VERSION=12.4.0
   > 
+ARROW_AZURE_STORAGE_BLOB_BUILD_SHA256_CHECKSUM=ce77055ff5e1b88826a89a29399ffbdcdc77beca1eae61c81f34a3f6e0a20715
   > ⬇️ Suggested change
   > -ARROW_AZURE_STORAGE_BLOB_BUILD_VERSION=12.4.0
   > 
-ARROW_AZURE_STORAGE_BLOB_BUILD_SHA256_CHECKSUM=ce77055ff5e1b88826a89a29399ffbdcdc77beca1eae61c81f34a3f6e0a20715
   > +ARROW_AZURE_STORAGE_BLOBS_BUILD_VERSION=12.4.0
   > 
+ARROW_AZURE_STORAGE_BLOBS_BUILD_SHA256_CHECKSUM=ce77055ff5e1b88826a89a29399ffbdcdc77beca1eae61c81f34a3f6e0a20715
   >
   > In cpp/src/arrow/filesystem/azurefs.cc:
   >> + if (this->is_azurite) {
   > + account_blob_url = "http://127.0.0.1:10000/"; + account_name + "/";
   > + account_dfs_url = "http://127.0.0.1:10000/"; + account_name + "/";
   > + } else {
   > + account_dfs_url = "https://"; + account_name + ".dfs.core.windows.net/";
   > + account_blob_url = "https://"; + account_name + ".blob.core.windows.net/";
   > + }
   > + storage_credentials_provider =
   > + 
std::make_shared<Azure::Storage::StorageSharedKeyCredential>(account_name,
   > + account_key);
   > + credentials_kind = AzureCredentialsKind::StorageCredentials;
   > + return Status::OK();
   > +}
   > +
   > +Status AzureOptions::ConfigureConnectionStringCredentials(
   > + const std::string& connection_string_uri) {
   > It seems that connection string isn't an URI: 
   > 
https://docs.microsoft.com/en-us/azure/storage/common/storage-configure-connection-string
   >
   > In cpp/src/arrow/filesystem/azurefs.cc:
   >> + // Ensure container exists
   > + ARROW_ASSIGN_OR_RAISE(bool container_exists, 
   > impl_->ContainerExists(path.container));
   > + if (!container_exists) {
   > + RETURN_NOT_OK(impl_->CreateContainer(path.container));
   > + }
   > + std::vector<std::string> parent_path_to_file;
   > +
   > + for (const auto& part : path.path_to_file_parts) {
   > + parent_path_to_file.push_back(part);
   > + RETURN_NOT_OK(impl_->CreateEmptyDir(path.container, 
parent_path_to_file));
   > + }
   > + return Status::OK();
   > + } else {
   > + // Check parent dir exists
   > + if (path.has_parent()) {
   > + AzurePath parent_path = path.parent();
   > ⬇️ Suggested change
   > - AzurePath parent_path = path.parent();
   > + auto parent_path = path.parent();
   >
   > In cpp/thirdparty/versions.txt:
   >> @@ -33,6 +33,16 @@ ARROW_AWS_C_COMMON_BUILD_VERSION=v0.6.9 
   >> 
ARROW_AWS_C_COMMON_BUILD_SHA256_CHECKSUM=928a3e36f24d1ee46f9eec360ec5cebfe8b9b8994fe39d4fa74ff51aebb12717
 
   >> ARROW_AWS_C_EVENT_STREAM_BUILD_VERSION=v0.1.5 
   >> 
ARROW_AWS_C_EVENT_STREAM_BUILD_SHA256_CHECKSUM=f1b423a487b5d6dca118bfc0d0c6cc596dc476b282258a3228e73a8f730422d4
   > +ARROW_AZURE_CORE_BUILD_VERSION=1.5.0
   > 
+ARROW_AZURE_CORE_BUILD_SHA256_CHECKSUM=dab2caa54d062b61dbe982e29a4f1fcc70216b51b038a807763712a40dd258e9
   > +ARROW_AZURE_IDENTITY_BUILD_VERSION=1.2.0
   > 
+ARROW_AZURE_IDENTITY_BUILD_SHA256_CHECKSUM=ad4702890c25f956c59a63be4571a08ae0690fa6d2bfbebf326d0fd2e9b72945
   > +ARROW_AZURE_STORAGE_BLOB_BUILD_VERSION=12.4.0
   > 
+ARROW_AZURE_STORAGE_BLOB_BUILD_SHA256_CHECKSUM=ce77055ff5e1b88826a89a29399ffbdcdc77beca1eae61c81f34a3f6e0a20715
   > +ARROW_AZURE_STORAGE_COMMON_BUILD_VERSION=12.2.3
   > 
+ARROW_AZURE_STORAGE_COMMON_BUILD_SHA256_CHECKSUM=2d58e9c314b1b32f7d09880239a4ecce6686ed6df236a58f681ae5d526ed6201
   > +ARROW_AZURE_STORAGE_FILES_DATALAKE_BUILD_VERSION=12.3.1
   > 
+ARROW_AZURE_STORAGE_FILES_DATALAKE_BUILD_SHA256_CHECKSUM=a5b74076a751d7cfaf7c56674a40ce2792c4fab9add18758fab1fe091d00baff
   > Could you update to the latest versions? I got segmentation fault with 
   > these versions on my Debian GNU/Linux sid environment.
   > diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake 
   > b/cpp/cmake_modules/ThirdpartyToolchain.cmake
   > index 3a555b0c4b..73d582a7a9 100644
   > --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
   > +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
   > @@ -4841,14 +4841,15 @@ macro(build_azuresdk) 
   > set(AZURESDK_COMMON_CMAKE_ARGS ${EP_COMMON_CMAKE_ARGS}
   > + "-DCMAKE_INSTALL_PREFIX=${AZURESDK_PREFIX}"
   > + "-DCMAKE_PREFIX_PATH=${AZURESDK_PREFIX}" -DBUILD_SHARED_LIBS=OFF 
   > -DCMAKE_INSTALL_LIBDIR=${AZURESDK_LIB_DIR}
   > + -DDISABLE_AZURE_CORE_OPENTELEMETRY=ON -DENABLE_TESTING=OFF 
   > -DENABLE_UNITY_BUILD=ON -DOPENSSL_ROOT_DIR=${OPENSSL_ROOT_HINT}
   > - -DWARNINGS_AS_ERRORS=OFF
   > - "-DCMAKE_INSTALL_PREFIX=${AZURESDK_PREFIX}"
   > - "-DCMAKE_PREFIX_PATH=${AZURESDK_PREFIX}")
   > + -DWARNINGS_AS_ERRORS=OFF) file(MAKE_DIRECTORY ${AZURESDK_INCLUDE_DIR}) 
   > diff --git a/cpp/thirdparty/versions.txt b/cpp/thirdparty/versions.txt
   > index a8797de94b..1f21385a78 100644
   > --- a/cpp/thirdparty/versions.txt
   > +++ b/cpp/thirdparty/versions.txt
   > @@ -33,14 +33,14 @@ ARROW_AWS_C_COMMON_BUILD_VERSION=v0.6.9 
   > 
ARROW_AWS_C_COMMON_BUILD_SHA256_CHECKSUM=928a3e36f24d1ee46f9eec360ec5cebfe8b9b8994fe39d4fa74ff51aebb12717
 
   > ARROW_AWS_C_EVENT_STREAM_BUILD_VERSION=v0.1.5 
   > 
ARROW_AWS_C_EVENT_STREAM_BUILD_SHA256_CHECKSUM=f1b423a487b5d6dca118bfc0d0c6cc596dc476b282258a3228e73a8f730422d4
   > -ARROW_AZURE_CORE_BUILD_VERSION=1.5.0
   > 
-ARROW_AZURE_CORE_BUILD_SHA256_CHECKSUM=dab2caa54d062b61dbe982e29a4f1fcc70216b51b038a807763712a40dd258e9
   > -ARROW_AZURE_IDENTITY_BUILD_VERSION=1.2.0
   > 
-ARROW_AZURE_IDENTITY_BUILD_SHA256_CHECKSUM=ad4702890c25f956c59a63be4571a08ae0690fa6d2bfbebf326d0fd2e9b72945
   > -ARROW_AZURE_STORAGE_BLOB_BUILD_VERSION=12.4.0
   > 
-ARROW_AZURE_STORAGE_BLOB_BUILD_SHA256_CHECKSUM=ce77055ff5e1b88826a89a29399ffbdcdc77beca1eae61c81f34a3f6e0a20715
   > -ARROW_AZURE_STORAGE_COMMON_BUILD_VERSION=12.2.3
   > 
-ARROW_AZURE_STORAGE_COMMON_BUILD_SHA256_CHECKSUM=2d58e9c314b1b32f7d09880239a4ecce6686ed6df236a58f681ae5d526ed6201
   > +ARROW_AZURE_CORE_BUILD_VERSION=1.7.1
   > 
+ARROW_AZURE_CORE_BUILD_SHA256_CHECKSUM=ae6f03e65d9773d11cf3b9619d0bc7f567272974cf31b9e1c8ca2fa0ea4fb4c6
   > +ARROW_AZURE_IDENTITY_BUILD_VERSION=1.3.0
   > 
+ARROW_AZURE_IDENTITY_BUILD_SHA256_CHECKSUM=46701acd8000f317d1c4b33263d5d3203924fadcfa5af4860ae9187046a72c45
   > +ARROW_AZURE_STORAGE_BLOB_BUILD_VERSION=12.5.0
   > 
+ARROW_AZURE_STORAGE_BLOB_BUILD_SHA256_CHECKSUM=12394d864144ced9fc3562ad48cfe3426604e871b5aa72853ca398e086f0c594
   > +ARROW_AZURE_STORAGE_COMMON_BUILD_VERSION=12.2.4
   > 
+ARROW_AZURE_STORAGE_COMMON_BUILD_SHA256_CHECKSUM=7644b4355b492ba2039236b9fd56c3e7bb80aad983d8bac6a731d74aaf64e03f
 
   > ARROW_AZURE_STORAGE_FILES_DATALAKE_BUILD_VERSION=12.3.1 
   > 
ARROW_AZURE_STORAGE_FILES_DATALAKE_BUILD_SHA256_CHECKSUM=a5b74076a751d7cfaf7c56674a40ce2792c4fab9add18758fab1fe091d00baff
 
   > ARROW_BOOST_BUILD_VERSION=1.75.0
   > —
   > Reply to this email directly, view it on GitHub, or unsubscribe.
   > You are receiving this because you were mentioned.Message ID: 
   > ***@***.***>
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to