Repository navigation
🎉 BigQuery Destination: improve check performance - #11350
Conversation
� Conflicts: � docs/integrations/destinations/snowflake.md
|
vmaltsev seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
github-actions
Bot
added
the
area/documentation
|
/test connector=connectors/destination-bigquery
|
# Conflicts: # docs/integrations/destinations/bigquery.md
| } | ||
| } | ||
|
|
||
| public AirbyteConnectionStatus checkStorageIamPermissions(JsonNode config) { |
There was a problem hiding this comment.
VitaliiMaltsev this is great, but could you put this inside the GCSDestination::check method instead? we will want this logic to be replicated for both GCS and BQ Destinatio
There was a problem hiding this comment.
once that is done I think we can move forward with this PR
There was a problem hiding this comment.
VitaliiMaltsev this is great, but could you put this inside the
GCSDestination::checkmethod instead? we will want this logic to be replicated for both GCS and BQ Destinatio
Sherif A. Nada (@sherifnada) Now the main idea in BigQuery check() method - to use credentials_json parameter for authentification and IAM storage permissions testing.
Storage storage = StorageOptions.newBuilder()
.setProjectId(config.get(BigQueryConsts.CONFIG_PROJECT_ID).asText())
**.setCredentials**(!isNull(credentials) ? credentials : ServiceAccountCredentials.getApplicationDefault())
.build().getService();
Restriction here that we can use only credentials_json but not hmac_key_access_id and hmac_key_secret. We already have this param for Destination BigQuery spec, so it's a valid case.
But if you take a look at spec.json for Destination GCS you'll find that authentification use hmac_key_access_id and hmac_key_secret and AWS S3 dependency. The easiest way is to just add the credentials_json parameter to the spec, but wouldn't that be a backwards compatibility issue for existing users?
WDYT?
There was a problem hiding this comment.
VitaliiMaltsev got it, thanks for clarifying. Created this issue: https://github.com/airbytehq/airbyte/issues/11398
There was a problem hiding this comment.
the approach in this PR is fine as an intermediate step
| } | ||
| } | ||
|
|
||
| public AirbyteConnectionStatus checkStorageIamPermissions(JsonNode config) { |
There was a problem hiding this comment.
VitaliiMaltsev got it, thanks for clarifying. Created this issue: https://github.com/airbytehq/airbyte/issues/11398
| } | ||
| } | ||
|
|
||
| public AirbyteConnectionStatus checkStorageIamPermissions(JsonNode config) { |
There was a problem hiding this comment.
the approach in this PR is fine as an intermediate step
…java/io/airbyte/integrations/destination/bigquery/BigQueryDestination.java
…check-perf' into vmaltsev/10377-bigquery-improve-check-perf
|
/publish connector=connectors/destination-bigquery
|
|
Sherif A. Nada (@sherifnada) VitaliiMaltsev this approach doesn't work when people are using conditional binding on roles/permissions. For instance if I grant the role |
4 participants
What
BiqQuery check method shows low performance
How
Test necessary permissions using Google Cloud CDK without any actions on GCS
Recommended reading order
BigQueryDestination.java🚨 User Impact 🚨
There should not be any user impact
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/SUMMARY.mddocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampledocs/integrations/README.mdairbyte-integrations/builds.mdAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described hereUpdating a connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described hereConnector Generator
-scaffoldin their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplatesthen checking in your changesTests
Unit
Put your unit tests output here.
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.