Skip to content

Add sample to schedule query with BQ DTS. #7703

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

Merged
merged 13 commits into from
Jun 6, 2019

Conversation

tswast
Copy link
Contributor

@tswast tswast commented Apr 12, 2019

Doesn't add tests. Because of the authorization_code parameter, I have no idea how to run automated tests.

@tswast tswast added api: bigquerydatatransfer Issues related to the BigQuery Data Transfer Service API. type: docs Improvement to the documentation for an API. labels Apr 12, 2019
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 12, 2019
@tswast tswast requested a review from shollyman April 12, 2019 23:52
@tswast tswast marked this pull request as ready for review April 12, 2019 23:52
@tswast tswast requested a review from crwilcox as a code owner April 12, 2019 23:52
@tswast
Copy link
Contributor Author

tswast commented Apr 13, 2019

Oh, and I'm pretty sure I had to use user-based credentials with this thing, but I haven't tried with a service account since I added the authorization_code parameter. (I have no idea how I'd get such a code for a service account.)

@tswast tswast changed the title Add semi-generated sample for BQ DTS. Add sample to schedule query with BQ DTS. Apr 16, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Apr 19, 2019
@tswast tswast requested a review from a team June 5, 2019 17:12
tswast added 3 commits June 5, 2019 10:37
This ensures the user (or service account) running the tests has owner permissions, which is required to run scheduled queries.
@tswast
Copy link
Contributor Author

tswast commented Jun 5, 2019

Wait for googleapis/synthtool#260 to merge.

@tswast tswast added do not merge Indicates a pull request not ready for merge, due to either quality or timing. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jun 5, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 5, 2019
@tswast tswast added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Jun 5, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 5, 2019
@tswast tswast added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 6, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 6, 2019
@tswast
Copy link
Contributor Author

tswast commented Jun 6, 2019

Unit test failure was related to #8147. I've reverted the generated code changes except for the noxfile update.

# authorization code to a value from the URL:
# https://www.gstatic.com/bigquerydatatransfer/oauthz/auth?client_id=433065040935-hav5fqnc9p9cht3rqneus9115ias2kn1.apps.googleusercontent.com&scope=https://www.googleapis.com/auth/bigquery%20https://www.googleapis.com/auth/drive&redirect_uri=urn:ietf:wg:oauth:2.0:oob
#
# authorization_code = "_4/ABCD-EFGHIJKLMNOP-QRSTUVWXYZ"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a live credential? Should we use something more abstract like AUTH_CODE_GOES_HERE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this a live credential?

No, that'd be pretty funny if it was (it's just the alphabet).

Should we use something more abstract like AUTH_CODE_GOES_HERE?

Maybe? I like when the example looks like the code I'm going to see, since it helps me with pattern matching, but I could be convinced that it's better to be clear that it's a placeholder.

"display_name": "Your Scheduled Query Name",
"data_source_id": "scheduled_query",
"params": {
"query": "SELECT 1; -- Use standard SQL syntax.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's show the injectable run params that DTS provides. The version of the sample I wrote did this:

SELECT
CURRENT_TIMESTAMP() as current_time,
@run_time as intended_run_time,
@run_date as intended_run_date,
17 as some_integer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 6b82d54.


@pytest.fixture(scope="module")
def credentials():
creds, _ = google.auth.default(["https://www.googleapis.com/auth/cloud-platform"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add some breadcrumbs about the permission setup for making the service account work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error messages actually showed me the commands to run this time, so that was good. Will the instructions change when it starts working how it's supposed to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@tswast tswast merged commit 0b23231 into googleapis:master Jun 6, 2019
@tswast tswast deleted the bqtransferservice-samples branch June 6, 2019 17:38
parthea pushed a commit that referenced this pull request Sep 22, 2023
* Add semi-generated sample for BQ DTS.

* Add authorization_code as parameter.

* Give scheduled query example a shorter filename.

* Add test for create scheduled query sample.

* Create dataset in sample tests.

This ensures the user (or service account) running the tests has owner permissions, which is required to run scheduled queries.

* Add samples nox session.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquerydatatransfer Issues related to the BigQuery Data Transfer Service API. cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love. type: docs Improvement to the documentation for an API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants