GH-35879: [C++] Bump bundled google-cloud-cpp to 2.12.0 #36119

kou · 2023-06-16T01:45:22Z

Rationale for this change

The version will fix #35318.

What changes are included in this PR?

Use the latest released version.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Closes: [C++] Bump bundled google-cloud-cpp #35879

github-actions · 2023-06-16T01:45:58Z

⚠️ GitHub issue #35879 has been automatically assigned in GitHub to PR creator.

kou · 2023-06-16T07:40:31Z

@coryan If we update bundled google-cloud-cpp to 2.12.0 from 2.8.0, our GCSFS tests are failed with "missing project id error":

https://github.com/apache/arrow/actions/runs/5287300615/jobs/9567623572?pr=36119#step:11:223

[ RUN      ] TestGCSFSGeneric.Empty
D:/a/arrow/arrow/cpp/src/arrow/filesystem/gcsfs_test.cc:304: Failure
Failed
'gcs_fs_->CreateDir(bucket_name, true)' failed with Invalid: google::cloud::Status(INVALID_ARGUMENT: missing project id error_info={reason=INVALID_ARGUMENT, domain=gcloud-cpp, metadata={gcloud-cpp.source.function=CreateBucket, gcloud-cpp.source.line=402, gcloud-cpp.source.filename=D:/a/arrow/arrow/build/cpp/google_cloud_cpp_ep-install/include/google/cloud/storage/client.h, gcloud-cpp.version=v2.12.0}}). Detail: [errno 22] Invalid argument

Do we need to update our GCSFS?

coryan · 2023-06-16T12:44:36Z

We need to update the tests. The client library now performs more validation, and the testbench is too forgiving. The test used to get away with a request that would have failed against production.

We need to set the GOOGLE_CLOUD_PROJECT environment variable to some value good for testing. Alternatively, we need to expand the GCSFS configuration to accept the project id as one of the (optional) parameters and use that in the test.

Should I send a separate PR or should we try to add these changes to this PR?

kou · 2023-06-16T20:36:40Z

Thanks!

We need to set the GOOGLE_CLOUD_PROJECT environment variable to some value good for testing. Alternatively, we need to expand the GCSFS configuration to accept the project id as one of the (optional) parameters and use that in the test.

If specifying the project ID from API is useful for Apache Arrow users, how about choosing the alternative approach? If it's only useful for testing, the GOOGLE_CLOUD_PROJECT environment variable approach may be enough.

Should I send a separate PR or should we try to add these changes to this PR?

Could you send a separate PR? I think that it's easier for you.
If the project ID related changes don't need google-cloud-cpp 1.12.0, the separated PR don't need to update bundled google-cloud-cpp. If they need, you can cherry-pick commits from this branch and we can close this PR.

It seems that this PR still has a static linking related problem:

https://github.com/apache/arrow/actions/runs/5287582940/jobs/9568250982?pr=36119#step:5:3381

rror: package or namespace load failed for ‘arrow’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/arrow/r/check/arrow.Rcheck/00LOCK-arrow/00new/arrow/libs/arrow.so':
  /arrow/r/check/arrow.Rcheck/00LOCK-arrow/00new/arrow/libs/arrow.so: undefined symbol: _ZN4absl12lts_2021110213cord_internal18cord_btree_enabledE

So I think that a PR without google-cloud-cpp update will be easy for you.

coryan · 2023-06-16T21:28:43Z

We need to set the GOOGLE_CLOUD_PROJECT environment variable to some value good for testing. Alternatively, we need to expand the GCSFS configuration to accept the project id as one of the (optional) parameters and use that in the
test.

If specifying the project ID from API is useful for Apache Arrow users, how about choosing the alternative approach?

No problem. Happy to do that.

Should I send a separate PR or should we try to add these changes to this PR?

Could you send a separate PR?

Gladly. It may take me until Tuesday, I am not working on Monday.

It seems that this PR still has a static linking related problem:

Ugh, it can be hard to debug the little library dependencies in Abseil. Good luck!

https://github.com/apache/arrow/actions/runs/5287582940/jobs/9568250982?pr=36119#step:5:3381
rror: package or namespace load failed for ‘arrow’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/arrow/r/check/arrow.Rcheck/00LOCK-arrow/00new/arrow/libs/arrow.so':
  /arrow/r/check/arrow.Rcheck/00LOCK-arrow/00new/arrow/libs/arrow.so: undefined symbol: _ZN4absl12lts_2021110213cord_internal18cord_btree_enabledE
So I think that a PR without google-cloud-cpp update will be easy for you.

Sounds good, I will give it a try.

kou · 2023-06-16T21:36:27Z

Thanks!

It may take me until Tuesday, I am not working on Monday.

No problem.

kou · 2023-06-21T00:48:11Z

It seems that the static linking problem is solved.

coryan · 2023-06-22T00:46:08Z

You may have noticed, I created a PR to help with the problems here.

### Rationale for this change This fixes #36227, originally motivated by the problems in #36119, but seems like a valuable feature in any case. ### What changes are included in this PR? - Refactor some code to make it testable. - Add a new `std::optional<std::string>` field to the `GcsOptions` class. ### Are these changes tested? Yes, I expanded the unit tests. ### Are there any user-facing changes? Yes. I updated the field documentation. If I missed some documentation please let me know. I am also not familiar with the steps required to update the Python wrappers, if there is some documentation to follow I would appreciate it. I can expand this PR or send a separate one, your call. * Closes: #36227 Authored-by: Carlos O'Ryan <coryan@google.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

kou · 2023-06-22T04:25:07Z

GH-36228 is merged and I've rebased on main.

assignUser · 2023-06-22T14:39:19Z

Quick reminder to also upload the new version to our artifactory!

kou · 2023-06-23T08:53:28Z

Oh. I forgot it. Thanks!

kou · 2023-06-26T21:44:48Z

+1

@paleolimbot @thisisnic This pull request changes the R part a bit. Could you review it?

See also the context: #36228

paleolimbot

Thank you for updating the R bits! I can't spot anything that wouldn't have been caught by the tests.

kou · 2023-06-27T20:55:04Z

Thanks!
I'll merge this.

raulcd · 2023-06-28T10:19:53Z

I am unsure if there's something else to be done but it seems the nightlies for:

Failed with:

E   pyarrow.lib.ArrowInvalid: google::cloud::Status(INVALID_ARGUMENT: missing project id error_info={reason=INVALID_ARGUMENT, domain=gcloud-cpp, metadata={gcloud-cpp.source.function=CreateBucket, gcloud-cpp.source.line=402, gcloud-cpp.source.filename=/build/cpp/google_cloud_cpp_ep-install/include/google/cloud/storage/client.h, gcloud-cpp.version=v2.12.0}}). Detail: [errno 22] Invalid argument

Is there something else to be done? Should I create a new issue for those failures?

coryan · 2023-06-28T12:13:30Z

I am unsure if there's something else to be done but it seems the nightlies for:

test-debian-11-python-3

test-fedora-35-python-3

Failed with:
E   pyarrow.lib.ArrowInvalid: google::cloud::Status(INVALID_ARGUMENT: missing project id error_info={reason=INVALID_ARGUMENT, domain=gcloud-cpp, metadata={gcloud-cpp.source.function=CreateBucket, gcloud-cpp.source.line=402, gcloud-cpp.source.filename=/build/cpp/google_cloud_cpp_ep-install/include/google/cloud/storage/client.h, gcloud-cpp.version=v2.12.0}}). Detail: [errno 22] Invalid argument
Is there something else to be done? Should I create a new issue for those failures?

I probably missed making changes for Python in #36228. I do not know where to start with these changes, if somebody can point me to the right documentation, I will give it a shot.

raulcd · 2023-06-28T13:00:35Z

Hi @coryan !
No worries, from my understanding we have to add the new project_id to our GcsFileSystem constructor:
https://github.com/apache/arrow/blob/main/python/pyarrow/_gcsfs.pyx#L83-L89
And adding that to the underlying CGcsOptions.
We migt have to update the __reduce__ function on that file and update the tests on python/pyarrow/tests/test_fs.py:
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_fs.py#L211
I've created #36352 to track it.

conbench-apache-arrow · 2023-07-01T07:54:30Z

Conbench analyzed the 6 benchmark runs on commit 4cfe9fab.

There were 7 benchmark results indicating a performance regression:

Commit Run on arm64-m6g-linux-compute at 2023-06-27 22:40:34Z
- params=, source=cpp-micro, suite=parquet-bloom-filter-benchmark
- params=, source=cpp-micro, suite=parquet-bloom-filter-benchmark
and 5 more (see the report linked below)

The full Conbench report has more details.

github-actions bot added awaiting committer review Awaiting committer review Component: C++ labels Jun 16, 2023

kou force-pushed the cpp-google-cloud-cpp branch from 5ef0e24 to ac35574 Compare June 19, 2023 08:03

kou added a commit to kou/arrow that referenced this pull request Jun 19, 2023

apacheGH-36119: [C++][Dev] Add support for using python3 to run IWYU

9dfe32f

github-actions bot mentioned this pull request Jun 19, 2023

GH-36157: [C++][Dev] Add support for using python3 to run IWYU #36159

Merged

github-actions bot assigned kou Jun 19, 2023

coryan mentioned this pull request Jun 22, 2023

GH-36227: [C++] New GcsOption to set the project id #36228

Merged

kou force-pushed the cpp-google-cloud-cpp branch from 7ed1805 to dc2c7b5 Compare June 22, 2023 04:24

kou requested review from assignUser, paleolimbot and thisisnic as code owners June 22, 2023 06:28

github-actions bot added the Component: R label Jun 22, 2023

kou force-pushed the cpp-google-cloud-cpp branch from 6a26e76 to a30087e Compare June 25, 2023 05:47

kou added 5 commits June 26, 2023 15:16

apacheGH-35879: [C++] Bump bundled google-cloud-cpp to 2.12.0

5a4aef2

Depend on absl::cord

6397590

Bundle absl::cord

a4fad9f

Bundle absl::cord_internal

0e14ec8

Bundle absl_cordz_functions

bca3dd9

kou added 13 commits June 26, 2023 15:16

Bundle absl::cordz_info

dd65810

Bundle absl::cordz_handle

f58791b

Bundle absl::synchronization

ec47832

Bundle absl::symbolize

2135936

Bundle absl::malloc_internal

456467d

Bundle absl::debugging_internal

a6d1003

Bundle absl::stacktrace

7b94780

Bundle absl::demangle_internal

495b1ec

Bundle absl::exponential_biased

1c86596

Fix

c050f09

Fold

51ebc7c

Add support for mirror

b08a715

Add project_id binding to R

e36a091

kou force-pushed the cpp-google-cloud-cpp branch from 4edcebf to e36a091 Compare June 26, 2023 06:25

Fix a typo

4da3753

paleolimbot approved these changes Jun 27, 2023

View reviewed changes

github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Jun 27, 2023

kou merged commit 4cfe9fa into apache:main Jun 27, 2023

kou deleted the cpp-google-cloud-cpp branch June 27, 2023 20:55

kou removed the awaiting merge Awaiting merge label Jun 27, 2023

raulcd mentioned this pull request Jun 28, 2023

[Python][CI] GCFS python tests fail due to updated version of google-cloud-cpp #36352

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-35879: [C++] Bump bundled google-cloud-cpp to 2.12.0 #36119

GH-35879: [C++] Bump bundled google-cloud-cpp to 2.12.0 #36119

kou commented Jun 16, 2023 •

edited

Loading

github-actions bot commented Jun 16, 2023

kou commented Jun 16, 2023

coryan commented Jun 16, 2023

kou commented Jun 16, 2023 •

edited

Loading

coryan commented Jun 16, 2023

kou commented Jun 16, 2023

kou commented Jun 21, 2023

coryan commented Jun 22, 2023

kou commented Jun 22, 2023

assignUser commented Jun 22, 2023

kou commented Jun 23, 2023

kou commented Jun 26, 2023 •

edited

Loading

paleolimbot left a comment

kou commented Jun 27, 2023

raulcd commented Jun 28, 2023

coryan commented Jun 28, 2023

raulcd commented Jun 28, 2023

conbench-apache-arrow bot commented Jul 1, 2023

GH-35879: [C++] Bump bundled google-cloud-cpp to 2.12.0 #36119

GH-35879: [C++] Bump bundled google-cloud-cpp to 2.12.0 #36119

Conversation

kou commented Jun 16, 2023 • edited Loading

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

github-actions bot commented Jun 16, 2023

kou commented Jun 16, 2023

coryan commented Jun 16, 2023

kou commented Jun 16, 2023 • edited Loading

coryan commented Jun 16, 2023

kou commented Jun 16, 2023

kou commented Jun 21, 2023

coryan commented Jun 22, 2023

kou commented Jun 22, 2023

assignUser commented Jun 22, 2023

kou commented Jun 23, 2023

kou commented Jun 26, 2023 • edited Loading

paleolimbot left a comment

Choose a reason for hiding this comment

kou commented Jun 27, 2023

raulcd commented Jun 28, 2023

coryan commented Jun 28, 2023

raulcd commented Jun 28, 2023

conbench-apache-arrow bot commented Jul 1, 2023

kou commented Jun 16, 2023 •

edited

Loading

kou commented Jun 16, 2023 •

edited

Loading

kou commented Jun 26, 2023 •

edited

Loading