search

treasure-data / digdag

Workload Automation System
https://www.digdag.io/
Apache License 2.0
1.3k stars 221 forks source link

Support location in bq operator #1767

Closed tkawachi closed 1 year ago

tkawachi commented 1 year ago

Set Job location for non-US location.

Fixes #1088

tkawachi commented 1 year ago

@yoyama can you review this PR?

yoyama commented 1 year ago

@tkawachi Thank you for your contribution. I left comments, please check them. In addition, adding integration tests is preferred. https://github.com/treasure-data/digdag/blob/master/digdag-tests/src/test/java/acceptance/td/BigQueryIT.java And we need to update the document as following. https://docs.digdag.io/operators/bq.html

tkawachi commented 1 year ago

Thank you for your review, @yoyama. I have added the integration test, but for the extract and load tests, the location of the BigQuery and GCS bucket must match. Currently, I suppose US bucket is set as GCS_TEST_BUCKET, but in addition to this, bucket of other location is needed. If you can create the bucket for me, I will write an integrated test for extract and load. Since I used asia-northeast1 in other tests, a bucket for asia-northeast1 might be desirable.

yoyama commented 1 year ago

@tkawachi Thank you for your fix. I created a bucket in asia region and set GCS_TEST_BUCKET_ASIA variable in CircleCI. Could you confirm it ?

tkawachi commented 1 year ago

@yoyama I wrote tests for load and extract. It assumes that GCS_TEST_BUCKET_ASIA is in asia-northeast1, not asia (BigQuery can't be located in asia). Where can I check the test result of CircleCI?

yoyama commented 1 year ago

@tkawachi I pushed your PR to digdag repo to run CircleCI. It failed as follows.

https://app.circleci.com/pipelines/github/treasure-data/digdag/718/workflows/237633a7-4be4-4697-8e66-45195f1505b3/jobs/4699/parallel-runs/1?filterBy=ALL

Could you confirm it ? The root cause is the setting of CircleCI and I fixed and confirmed BigQueryIT* passed. I faced still a failure but it is not related to this PR.

yoyama commented 1 year ago

@tkawachi I add a minor comment. Could you confirm it ?

yoyama commented 1 year ago

@tkawachi I have an another ask. Could you update the doc on the new parameters ? The doc on BigQuery is managed in digdag-docs/src/operators/

tkawachi commented 1 year ago

I updated the docs. I didn't change bq_ddl.md since it already mentions about location.

toru-takahashi commented 1 year ago

Hello @tkawachi

Thank you for your great PR and contribution to digdag! We are really appreciated with your efforts.

We would like to deliver some TD/Digdag swag to you as a token of our appreciation. If you don't mind, could you fill your address in the following Google form? So that we can send to you.

https://forms.gle/37jhNBFJo9kyLvLx9