Skip to content
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

Support location in bq operator #1767

Merged
merged 6 commits into from
Oct 17, 2022

Conversation

tkawachi
Copy link
Contributor

Set Job location for non-US location.

Fixes #1088

@tkawachi
Copy link
Contributor Author

@yoyama can you review this PR?

@yoyama
Copy link
Contributor

yoyama commented Sep 26, 2022

@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
Copy link
Contributor Author

tkawachi commented Oct 2, 2022

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
Copy link
Contributor

yoyama commented Oct 7, 2022

@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
Copy link
Contributor Author

tkawachi commented Oct 8, 2022

@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
Copy link
Contributor

yoyama commented Oct 11, 2022

@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
Copy link
Contributor

yoyama commented Oct 11, 2022

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

@yoyama
Copy link
Contributor

yoyama commented Oct 12, 2022

@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
Copy link
Contributor Author

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

Copy link
Contributor

@yoyama yoyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Thank you for your contribution!

@yoyama yoyama merged commit f328ed9 into treasure-data:master Oct 17, 2022
@toru-takahashi
Copy link
Contributor

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

@mishida mishida mentioned this pull request Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could not find BigQuery jobs in not US location
3 participants