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

docs(bigquery): update existing code samples #9212

Merged
merged 4 commits into from
Sep 26, 2019

Conversation

emar-kar
Copy link
Contributor

List of changes:

  1. browse_table_data:
    • add a description of how to print row data in tabular format. (as it was before).
  2. All files
    • add API request, Must match the destination dataset location and Waits for the job to complete comments.
  3. get_routine:
    • change whitespaces in prints to \t;
    • since routine.arguments doesn't have type_ attribute, it was changed to data_type.
    for argument in routine.arguments:
        print("\t\tName: '{}'".format(argument.name))
        print("\t\tType: '{}'".format(argument.type_))
        AttributeError: 'RoutineArgument' object has no attribute 'type_'
  1. test_browse_table_data:
    • add additional assertions (see point 1).
  2. test_load_table_dataframe:
    • according to pytest docs .importorskip should be assigned to its canonical name (@tswast can you confirm these imports are required).
  3. test_get_dataset and test_list_datasets_by_label:
    • exclude string formatting from the assert.
  4. test_routine_samples:
    • divide into separate test files:
      • test_create_routine;
      • test_create_routine_ddl;
      • test_delete_routine;
      • test_get_routine (new file);
      • test_list_routines;
      • test_update_routine;

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 11, 2019
@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 11, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 11, 2019
@IlyaFaer IlyaFaer marked this pull request as ready for review September 11, 2019 14:38
@IlyaFaer IlyaFaer requested review from a team and removed request for a team September 11, 2019 14:38
@plamut plamut added the api: bigquery Issues related to the BigQuery API. label Sep 12, 2019
Copy link
Contributor

@plamut plamut left a comment

Choose a reason for hiding this comment

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

Just wondering - what's with all the dots at the end of one-line comments, e.g. # API request.?
It seems a bit unusual (to me at lest), but nothing significant.

Anyhow, the changes look good to me all in all.

@plamut plamut added the type: cleanup An internal cleanup or hygiene concern. label Sep 19, 2019
@emar-kar
Copy link
Contributor Author

@plamut Yeah, I was surprised too. I saw these dots in several files that were already there. I need to know for sure, how @tswast would like to see BigQuery samples, to unify the design of all files. I appreciate your review.

@tswast
Copy link
Contributor

tswast commented Sep 24, 2019

The dots are probably because our internal samples style requires that all comments be complete sentences, with capital letters and punctuation. Accordingly, these comments should be changed to "Make an API request." or "The following function makes an API request."

@plamut plamut changed the title BigQuery: Update existing samples docs(bigquery): update existing code samples Sep 25, 2019
@plamut
Copy link
Contributor

plamut commented Sep 25, 2019

@emar-kar I presume you just rebased, correct, no other changes?

If this was to fix the commit message lint check, I changed the PR title to make the bot happy. We can merge the PR once the tests pass again.

@emar-kar
Copy link
Contributor Author

@plamut yeah, I changed the comment lines and got this confusing error. New day = new features. Just read all about it. Thank you!

@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 25, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 25, 2019
@plamut
Copy link
Contributor

plamut commented Sep 25, 2019

The Recommender failure is probably flakiness, I saw it on one of the other PRs, too (a re-run helped).

Edit: Well, or a "permanent" issue until it gets fixed, it again failed on several PRs even after retries.

@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 25, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 25, 2019
@tswast tswast merged commit 21cf56e into googleapis:master Sep 26, 2019
@emar-kar emar-kar deleted the update-existing-samples branch October 16, 2019 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants