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

feat: add support for Parquet options #679

Merged
merged 4 commits into from Jun 2, 2021
Merged

Conversation

@plamut
Copy link
Contributor

@plamut plamut commented May 25, 2021

Closes #661.

For load jobs and external tables config.

PR checklist:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)
For load jobs and external tables config.
@plamut plamut requested a review from shollyman May 25, 2021
@plamut plamut requested review from as code owners May 25, 2021
@google-cla google-cla bot added the cla: yes label May 25, 2021
google/cloud/bigquery/format_options.py Outdated Show resolved Hide resolved
Loading
@google-cla
Copy link

@google-cla google-cla bot commented May 25, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Loading

@google-cla google-cla bot added cla: no and removed cla: yes labels May 25, 2021
@plamut
Copy link
Contributor Author

@plamut plamut commented May 25, 2021

We still need to expose ParquetOptions in the google.cloud.bigquery namespace to be consistent with enums, etc.

Loading

@tseaver
Copy link
Contributor

@tseaver tseaver commented May 25, 2021

@googlebot I fixed it.

Loading

@google-cla
Copy link

@google-cla google-cla bot commented May 25, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Loading

1 similar comment
@google-cla
Copy link

@google-cla google-cla bot commented May 26, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Loading

@plamut
Copy link
Contributor Author

@plamut plamut commented May 26, 2021

Exposed the config object in google.cloud.bigquery namespace and also made sure that parquet_options works well with the options attribute.

Loading

@simonvanderveldt
Copy link

@simonvanderveldt simonvanderveldt commented Jun 1, 2021

@plamut My team is currently a bit blocked because this functionality is missing from the BigQuery Python library. Is there anything we can do to help this get merged and released? Maybe some testing?

Loading

@plamut
Copy link
Contributor Author

@plamut plamut commented Jun 1, 2021

@simonvanderveldt Thanks for bumping this, it appears that it's incorrectly blocked by the CLA bot (and still needs one approval).

@shollyman Can you please convince the bot to change its mind? :) And add a review, if possible?

Is there anything we can do to help this get merged and released? Maybe some testing?

If you want, you can test the BigQuery client with this PR branch, but primarily someone from Google with sufficient permissions needs to unblock the PR first.

Loading

@simonvanderveldt
Copy link

@simonvanderveldt simonvanderveldt commented Jun 1, 2021

If you want, you can test the BigQuery client with this PR branch, but primarily someone from Google with sufficient permissions needs to unblock the PR first.

Just tested it, working as expected :) We only need the list inference and using the library from this PR gives the same results as when using bq load --parquet_enable_list_inference.

Loading

@googlebot
Copy link

@googlebot googlebot commented Jun 1, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Loading

@stephaniewang526
Copy link
Member

@stephaniewang526 stephaniewang526 commented Jun 1, 2021

Looks like we're missing CLA signing from tseaver@palladion.com

Loading

@plamut
Copy link
Contributor Author

@plamut plamut commented Jun 1, 2021

But Tres is a well-known contributor? Or is it because a different email address was used?

Loading

@busunkim96
Copy link
Contributor

@busunkim96 busunkim96 commented Jun 1, 2021

@plamut I looks like there is no CLA for tseaver@palladion.com. @tseaver's CLA lists tres.seaver@gmail.com.

I think if you edit the Co-authored-by: Tres Seaver <tseaver@palladion.com> in the commit body the CLA bot will be appeased. Please holler if that doesn't work.

Loading

@google-cla
Copy link

@google-cla google-cla bot commented Jun 2, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Loading

@google-cla google-cla bot added cla: yes and removed cla: no labels Jun 2, 2021
@plamut
Copy link
Contributor Author

@plamut plamut commented Jun 2, 2021

That trick with editing the commit message worked, great! Since we also have an approval now, I'll merge and release this today.

Loading

@plamut
Copy link
Contributor Author

@plamut plamut commented Jun 2, 2021

@busunkim96 As for the CLA bot, isn't this a bit fragile? Theoretically one could take another person's work posted as a suggestion, edit out the line in the commit message linking to that person, and then merge a non-CLA code into the project?

We can discuss this offline.

Loading

@plamut plamut merged commit d792ce0 into googleapis:master Jun 2, 2021
11 checks passed
Loading
@plamut plamut deleted the iss-661 branch Jun 2, 2021
@plamut
Copy link
Contributor Author

@plamut plamut commented Jun 2, 2021

@simonvanderveldt The changes have just been released, happy coding!

Loading

@simonvanderveldt
Copy link

@simonvanderveldt simonvanderveldt commented Jun 2, 2021

@plamut Awesome! Thanks a lot for the quick merge and release!

Loading

@tseaver
Copy link
Contributor

@tseaver tseaver commented Jun 7, 2021

@busunkim96 literally all of my commits to Google's repositories over the last seven years have been signed with the tseaver@palladion.com email address. The CLAbot seems flaky / fragile for commits which are merges of through-the-web suggestions.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants