BigQuery: Add dry_run option to BigQuery magic by shubha-rajan · Pull Request #9067 · googleapis/google-cloud-python

shubha-rajan · 2019-08-21T01:37:38Z

See #8143
Adding the --dry_run flag returns a QueryJob instead of a pandas DataFrame as requested in the original issue. The QueryJob can also be stored in a variable if the destination_var magic argument is present

…nt, a QueryJob object is returned for inspection instead of an empty DataFrame

plamut

Thanks for the PR! It generally it looks good to me, apart from the dry_run argument help string.

I noticed, however, than when using the dry_run mode, an error is printed to the console:

In [16]: %%bigquery --dry_run 
    ...: SELECT * FROM `bigquery-public-data.samples.shakespeare` LIMIT 5; 
    ...:  
    ...:                                                                                                                                                                                                                                                                                                                                                                                                                          
Executing query with job ID: None

ERROR:
 404 GET https://www.googleapis.com/bigquery/v2/projects/precise-truck-742/queries/None?maxResults=0&location=US: Not found: Job precise-truck-742:US.None

(job ID: None)

                   -----Query Job SQL Follows-----                    

    |    .    |    .    |    .    |    .    |    .    |    .    |
   1:SELECT * FROM `bigquery-public-data.samples.shakespeare` LIMIT 5;
   2:
    |    .    |    .    |    .    |    .    |    .    |    .    |

This can be confusing for the users, and would be good to have it fixed. However, if fixing _run_query() proves to be too complex in the scope of this PR, we can make these changes separately.

bigquery/google/cloud/bigquery/magics.py

Co-Authored-By: Peter Lamut <plamut@users.noreply.github.com>

shubha-rajan · 2019-08-21T17:33:08Z

I'll investigate that error being printed out. I agree that it would be confusing to users, and don't think it's outside of the scope of the PR

tswast · 2019-08-21T21:41:37Z

When you run a dry run query, a job is not actually created, so fetching the results fails.

…jan/google-cloud-python into bq-add-dryrun-to-magic

tswast · 2019-08-21T22:03:15Z

Have you run this in a notebook, yet? I'm curious what the output looks like. We probably want to improve the __str__ or __repr__ functions on QueryJob to at least show estimated_bytes_processed, because that's what people running dryrun queries are going to care about.

shubha-rajan · 2019-08-22T01:08:49Z

@tswast as of now, running it in a notebook is silent. I tried modifying _run_query() and that made the error @plamut was referring to go away, but I still couldn't print out the estimated bytes without it clearing. I think the next step is to look at modifying those functions in QueryJob like you said

This reverts commit e3107f6.

…s total processed bytes to console

shubha-rajan · 2019-08-22T07:18:37Z

6efa408 allows the cell to output the total bytes processed. adding a variable argument allows the user to inspect values the returned QueryJob's properties including total_bytes_processed and total_bytes_billed

plamut

The new changes look good IMO.

I still have two questions, though:

If destination_var is used and an error occurs, should the failed query job still be stored for introspection? Currently it isn't:

In [179]: %%bigquery result --dry_run 
    ...: SELECT SELECT * FROM `bigquery-public-data.samples.shakespeare` LIMIT 5; 
    ...:  
    ...:  
    ...:                                                                                                                                                                                                                                                                                                                                                                                                                         

ERROR:
400 POST https://www.googleapis.com/bigquery/v2/projects/precise-truck-742/jobs: Syntax error: Unexpected keyword SELECT at [1:8]

In [180]: "result" in locals()                                                                                                                                                                                                                                                                                                                                                                                                    
Out[180]: False

If destination_var is not specified and an error occurs, it would IMO still be useful to print a query even on dry runs, especially on syntax errors. Currently this information is omitted in dry runs, is this intentional?

In [188]: %%bigquery --dry_run 
    ...: SELECT SELECT * FROM `bigquery-public-data.samples.shakespeare` LIMIT 5; 
    ...:  
    ...:  
    ...:                                                                                                                                                                                                                                                                                                                                                                                                                         

ERROR:
400 POST https://www.googleapis.com/bigquery/v2/projects/precise-truck-742/jobs: Syntax error: Unexpected keyword SELECT at [1:8]

Without the --dry-run option, the query gets printed as a part of the error message.

…variable in magics

shubha-rajan · 2019-08-23T05:55:38Z

So currently, destination_var doesn't return anything if an error occurs by default, even when the --dry_run flag isn't present. I investigated adding in that functionality, but the problem is that if calling _run_query raises an exception, it won't return a QueryJob. I suspect getting around that would mean making changes to exception handling in QueryJob and/or Client and idk what might end up breaking because of that 😅

As for printing out the SQL queries along with error messages, that's working as of the last commit.

plamut

So currently, destination_var doesn't return anything if an error occurs by default, even when the --dry_run flag isn't present.

Fine then, the --dry_run option does not have to deal with that, either. 👍

The error output is now much more useful, looks good from my side.

tswast

Looking great! Love the new error message behavior.

FYI: I've filed #9091 for consideration of how to handle failures with destination_var.

bigquery/google/cloud/bigquery/job.py

bigquery/tests/unit/test_magics.py

Co-Authored-By: Tim Swast <swast@google.com>

* added dry_run option to bigquery magics. when --dry_run flag is present, a QueryJob object is returned for inspection instead of an empty DataFrame * print estimated bytes instead of total bytes * updated docstring for _AsyncJob._begin * Update docstring for QueryJob._begin * added SQL query to error output and messaging for failure to save to variable in magics Co-Authored-By: Peter Lamut <plamut@users.noreply.github.com> Co-Authored-By: Tim Swast <swast@google.com>

shubha-rajan added 2 commits August 20, 2019 17:40

added dry_run option to bigquery magics. when --dry_run flag is prese…

5dfcc10

…nt, a QueryJob object is returned for inspection instead of an empty DataFrame

formatted for PEP8 compliance

7ce1ef4

shubha-rajan requested a review from a team August 21, 2019 01:37

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 21, 2019

plamut changed the title ~~Added dry_run option to bigquery magic~~ BigQuery: Add dry_run option to BigQuery magic Aug 21, 2019

plamut added the api: bigquery Issues related to the BigQuery API. label Aug 21, 2019

plamut suggested changes Aug 21, 2019

View reviewed changes

bigquery/google/cloud/bigquery/magics.py Outdated Show resolved Hide resolved

Edited dry_run help string

7d3f4e9

Co-Authored-By: Peter Lamut <plamut@users.noreply.github.com>

shubha-rajan added 2 commits August 21, 2019 14:57

fixed output for magic dry_run option

9580faa

Merge branch 'bq-add-dryrun-to-magic' of https://github.com/shubha-ra…

7f31051

…jan/google-cloud-python into bq-add-dryrun-to-magic

print estimated bytes instead of total bytes

e3107f6

shubha-rajan added 3 commits August 21, 2019 18:26

Revert "print estimated bytes instead of total bytes"

3336697

This reverts commit e3107f6.

running the bigquery magic in a notebook with the dryrun option print…

6efa408

…s total processed bytes to console

running the bigquery magic in a notebook with the dryrun option print…

1a9f187

…s total processed bytes to console

shubha-rajan force-pushed the bq-add-dryrun-to-magic branch 2 times, most recently from 1a9f187 to 6efa408 Compare August 22, 2019 07:11

plamut reviewed Aug 22, 2019

View reviewed changes

shubha-rajan added 3 commits August 22, 2019 16:26

added SQL query to error output and messaging for failure to save to …

c301b0f

…variable in magics

fixed broken tests and coverage failure

50d5f72

resolve merge

106f115

plamut approved these changes Aug 23, 2019

View reviewed changes

tswast mentioned this pull request Aug 23, 2019

BigQuery: [magics] if destination_var is set and an error occurs, save the QueryJob for further inspection #9091

Closed

tswast requested changes Aug 23, 2019

View reviewed changes

bigquery/google/cloud/bigquery/job.py Outdated Show resolved Hide resolved

bigquery/google/cloud/bigquery/job.py Outdated Show resolved Hide resolved

bigquery/tests/unit/test_magics.py Outdated Show resolved Hide resolved

bigquery/tests/unit/test_magics.py Outdated Show resolved Hide resolved

plamut mentioned this pull request Aug 23, 2019

BigQuery: Convert Sphinx-style docstrings to Google style #9092

Closed

shubha-rajan and others added 8 commits August 23, 2019 10:12

Update docstring for QueryJob._begin

617ea00

Co-Authored-By: Tim Swast <swast@google.com>

removed result variable from dryrun tests

d6931b0

merged in docstring changes

e7a0371

updated docstring for _AsyncJob._begin

16e3127

fixed bug in dryrun test

dacbd2a

removed trailing whitespace to make linter happy

f0eab53

fix docstring format

425671b

fix docstring

f5877c0

tswast approved these changes Aug 23, 2019

View reviewed changes

tswast merged commit 0f6fa6b into googleapis:master Aug 23, 2019

tswast mentioned this pull request Aug 23, 2019

BigQuery: Add dryrun option to magic. #8143

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BigQuery: Add dry_run option to BigQuery magic#9067

BigQuery: Add dry_run option to BigQuery magic#9067
tswast merged 20 commits intogoogleapis:masterfrom
shubha-rajan:bq-add-dryrun-to-magic

shubha-rajan commented Aug 21, 2019

Uh oh!

plamut left a comment •

edited

Loading

Uh oh!

Uh oh!

shubha-rajan commented Aug 21, 2019

Uh oh!

tswast commented Aug 21, 2019

Uh oh!

tswast commented Aug 21, 2019

Uh oh!

shubha-rajan commented Aug 22, 2019 •

edited

Loading

Uh oh!

shubha-rajan commented Aug 22, 2019

Uh oh!

plamut left a comment •

edited

Loading

Uh oh!

shubha-rajan commented Aug 23, 2019

Uh oh!

plamut left a comment •

edited

Loading

Uh oh!

tswast left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

shubha-rajan commented Aug 21, 2019

Uh oh!

plamut left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shubha-rajan commented Aug 21, 2019

Uh oh!

tswast commented Aug 21, 2019

Uh oh!

tswast commented Aug 21, 2019

Uh oh!

shubha-rajan commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shubha-rajan commented Aug 22, 2019

Uh oh!

plamut left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubha-rajan commented Aug 23, 2019

Uh oh!

plamut left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tswast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

plamut left a comment •

edited

Loading

shubha-rajan commented Aug 22, 2019 •

edited

Loading

plamut left a comment •

edited

Loading

plamut left a comment •

edited

Loading