kou commented on code in PR #12763:
URL: https://github.com/apache/arrow/pull/12763#discussion_r854771064


##########
ci/scripts/python_test.sh:
##########
@@ -47,9 +48,13 @@ export PYARROW_TEST_CUDA
 export PYARROW_TEST_DATASET
 export PYARROW_TEST_FLIGHT
 export PYARROW_TEST_GANDIVA
+export PYARROW_TEST_GCS
 export PYARROW_TEST_HDFS
 export PYARROW_TEST_ORC
 export PYARROW_TEST_PARQUET
 export PYARROW_TEST_S3
 
+if [ -f "/arrow/ci/scripts/install_gcs_testbench.sh" ]; then
+  /arrow/ci/scripts/install_gcs_testbench.sh default
+fi

Review Comment:
   Do we need this?
   Our base Docker images such as 
https://github.com/apache/arrow/blob/master/ci/docker/ubuntu-22.04-cpp.dockerfile#L143
 already run this.
   If we have some Docker images that the script isn't ran, we can run the 
script in these Docker images.



##########
dev/tasks/conda-recipes/arrow-cpp/build-pyarrow.sh:
##########
@@ -18,6 +18,7 @@ else
     export PYARROW_WITH_GANDIVA=1
 fi
 export PYARROW_WITH_HDFS=1
+export PYARROW_WITH_GCS=1

Review Comment:
   Could you swap these lines to keep order?



##########
dev/tasks/tasks.yml:
##########
@@ -431,8 +431,8 @@ tasks:
 {############################## Wheel OSX ####################################}
 
 # enable S3 support from macOS 10.13 so we don't need to bundle curl, crypt 
and ssl
-{% for macos_version, macos_codename, arrow_s3 in [("10.9", "mavericks", 
"OFF"),
-                                                   ("10.13", "high-sierra", 
"ON")] %}
+{% for macos_version, macos_codename, arrow_s3, arrow_gcs in [("10.9", 
"mavericks", "OFF", "OFF"),
+                                                              ("10.13", 
"high-sierra", "ON", "OFF")] %}

Review Comment:
   Is it OK that GCS is `OFF` for `10.13` too?



##########
ci/scripts/python_wheel_unix_test.sh:
##########
@@ -67,6 +69,10 @@ import pyarrow.orc
 import pyarrow.parquet
 import pyarrow.plasma
 "
+  if [ "${PYARROW_TEST_GCS}" == "ON" ]; then
+    pip install 
https://github.com/googleapis/storage-testbench/archive/v0.16.0.tar.gz

Review Comment:
   Can we use `PYTHON=python ${source_dir}/ci/scripts/install_gcs_testbench.sh 
default` here?



##########
cpp/src/arrow/filesystem/gcsfs.h:
##########
@@ -27,11 +27,38 @@
 namespace arrow {
 namespace fs {
 
-struct GcsCredentials;
+// Opaque wrapper for GCS's library credentials to avoid exposing in Arrow 
headers.
+struct GcsCredentialsHolder;
+class GcsFileSystem;
+
+/// \brief Container for GCS Credentials and information necessary to recreate 
them.
+class GcsCredentials {

Review Comment:
   It seems that `ARROW_EXPORT` is missing.



##########
ci/scripts/python_wheel_unix_test.sh:
##########
@@ -67,6 +69,10 @@ import pyarrow.orc
 import pyarrow.parquet
 import pyarrow.plasma
 "
+  if [ "${PYARROW_TEST_GCS}" == "ON" ]; then
+    pip install 
https://github.com/googleapis/storage-testbench/archive/v0.16.0.tar.gz

Review Comment:
   Could you move this to `if [ "${CHECK_UNITTESTS}" == "ON" ]; then` block 
because this is needed only for unit test?



-- 
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