ARROW-15085: [C++] support credential types in GcsFileSystem #11945
Conversation
pitrou
left a comment
pitrou
left a comment
There was a problem hiding this comment.
Thanks for this. A couple suggestions, otherwise LGTM.
cpp/src/arrow/filesystem/gcsfs.h
Outdated
| /// These credentials are useful when using an out-of-band mechanism to fetch access | ||
| /// tokens. Note that access tokens are time limited, you will need to manually refresh | ||
| /// the tokens created by the out-of-band mechanism. | ||
| static GcsOptions AccessToken(const std::string& access_token, |
cpp/src/arrow/filesystem/gcsfs.h
Outdated
| /// Service account impersonation allows one principal (a user or service account) to | ||
| /// impersonate a service account. It requires that the calling principal has the | ||
| /// necessary permissions *on* the service account. | ||
| static GcsOptions ImpersonateServiceAccount( |
There was a problem hiding this comment.
Perhaps FromServiceAccount or FromImpersonatedServiceAccount?
cpp/src/arrow/filesystem/gcsfs.h
Outdated
| /// obtained from a Cloud Secret Manager or a similar service. | ||
| /// | ||
| /// [aip/4112]: https://google.aip.dev/auth/4112 | ||
| static GcsOptions ServiceAccountCredentials(const std::string& json_object); |
cpp/src/arrow/filesystem/gcsfs.cc
Outdated
| return options; | ||
| } | ||
|
|
||
| class GcsCredentialsProvider { |
There was a problem hiding this comment.
Nit, but it would probably be enough to make this a simple struct with a public member. The attribute wrapping doesn't seem very useful here.
|
I also realized I was not using this new option, fixed that too. |
|
Benchmark runs are scheduled for baseline = e63ba1c and contender = e1da4c9. e1da4c9 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
No description provided.