feat: Add support for quota project #225
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
nickcaballero
force-pushed
the
feat/quota-project
branch
from
44fd5e8 to
239071e
Compare
jayehwhyehentee
requested review from
jayehwhyehentee and
prashastia
and removed request for
jayehwhyehentee
jayehwhyehentee
left a comment
jayehwhyehentee
left a comment
There was a problem hiding this comment.
Thank you for raising this PR @nickcaballero. I have left a few minor comments. Can you share results from any actual invocation of this new functionality that demonstrates that it works as expected?
| if (credentials instanceof QuotaProjectIdProvider) { | ||
| return ((QuotaProjectIdProvider) credentials).getQuotaProjectId(); | ||
| } |
There was a problem hiding this comment.
Is this needed? Credentials are passed to the BQ client, and if they contain quota project, then doesn't the BQ client acknowledge them?
| .setCredentialsProvider( | ||
| FixedCredentialsProvider.create(options.getCredentials())) | ||
| .setCredentialsProvider(FixedCredentialsProvider.create(credentials)) | ||
| .setQuotaProjectId(quotaProjectId) |
There was a problem hiding this comment.
For clarity, please set quotaProjectId only if it is not null
| Credentials credentials = options.getCredentials(); | ||
| quotaProjectId = BigQueryUtils.getQuotaProjectId(options, credentials); |
There was a problem hiding this comment.
These variables should only be used once, so no value in storing them. Please revert to in-place retrievals.
| Credentials credentials = options.getCredentials(); | ||
| String quotaProjectId = BigQueryUtils.getQuotaProjectId(options, credentials); | ||
| BigQueryWriteSettings.Builder settingsBuilder = | ||
| BigQueryWriteSettings.newBuilder() | ||
| .setCredentialsProvider( | ||
| FixedCredentialsProvider.create(options.getCredentials())) | ||
| .setCredentialsProvider(FixedCredentialsProvider.create(credentials)) | ||
| .setQuotaProjectId(quotaProjectId) |
There was a problem hiding this comment.
Same as previous comments
| Credentials credentials = options.getCredentials(); | ||
| String quotaProjectId = BigQueryUtils.getQuotaProjectId(options, credentials); | ||
| bigQuery = | ||
| BigQueryOptions.newBuilder() | ||
| .setCredentials(options.getCredentials()) | ||
| .setCredentials(credentials) | ||
| .setQuotaProjectId(quotaProjectId) |
There was a problem hiding this comment.
Same as previous comments
2 participants
This PR adds support for setting the quota project used when connecting to BigQuery. By default, the project where the dataset lives is used, but some teams use quota projects to separate resources from billing.