-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix BigQuery configuration with parent-project-id #23041
Conversation
plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/CredentialsOptionsConfigurer.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/BaseBigQueryConnectorTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/BaseBigQueryConnectorTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/BaseBigQueryConnectorTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/BaseBigQueryConnectorTest.java
Outdated
Show resolved
Hide resolved
6240fb8
to
fcdb2cc
Compare
fcdb2cc
to
1698700
Compare
nit: typo in |
1698700
to
327d4ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please squash 4th and 3rd commits into one.
plugin/trino-bigquery/src/main/java/io/trino/plugin/bigquery/BigQueryClient.java
Show resolved
Hide resolved
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryParentProjectId.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryParentProjectId.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryParentProjectId.java
Outdated
Show resolved
Hide resolved
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryParentProjectId.java
Outdated
Show resolved
Hide resolved
/test-with-secrets sha=56ec6f35fda5235416262aab2120a25bdaf304ca |
The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/10567518374 |
56ec6f3
to
b1c326c
Compare
Big Query tests with secretes passed in mirror PR #23141 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be very helpful if we can have a short comment somewhere explaining the differences between project id, parent project id and how those are related to the quota/billing project.
Code itself seems fine but the semantics are unclear to me.
b1c326c
to
0c6611b
Compare
Quota project id is different name for parent project id, both indicates project used for quota and billing purposes. |
plugin/trino-bigquery/src/test/java/io/trino/plugin/bigquery/TestBigQueryParentProjectId.java
Outdated
Show resolved
Hide resolved
0c6611b
to
3f41661
Compare
3f41661
to
a076b11
Compare
@pajaks Thanks for writing the release note entry. I think users can't understand what was the issue from this sentence. Could you update it? We usually use "Fix failure when ..." for bug fix entries. |
Description
If
bigquery.parent-project-id
was specified in configuration it was set asprojectId
inBigQueryOptions
.For queries without
TableId
orDatasetId
specifiedBigQueryOptions
was used resulting in using wrong projectId.This problem is visible when using PTF with parent-proejct-id set, resulting in permission problem like:
Billing project id should be set using `setQuotaProjectId' according to https://cloud.google.com/java/docs/reference/google-cloud-core/latest/com.google.cloud.ServiceOptions#com_google_cloud_ServiceOptions_getQuotaProjectId__
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: