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 review from shollyman and a team May 25, 2021 14:36
@plamut plamut requested a review from a team as a code owner May 25, 2021 14:36
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label May 25, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 25, 2021
@google-cla
Copy link

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.

@google-cla google-cla bot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels May 25, 2021
@plamut
Copy link
Contributor Author

plamut commented May 25, 2021

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

@plamut plamut added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 25, 2021
@tseaver
Copy link
Contributor

tseaver commented May 25, 2021

@googlebot I fixed it.

@google-cla
Copy link

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.

1 similar comment
@google-cla
Copy link

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.

@plamut plamut removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 26, 2021
@plamut
Copy link
Contributor Author

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.

@simonvanderveldt
Copy link

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?

@plamut
Copy link
Contributor Author

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.

@simonvanderveldt
Copy link

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.

@googlebot
Copy link

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.

@stephaniewang526
Copy link
Contributor

Looks like we're missing CLA signing from [email protected]

@plamut
Copy link
Contributor Author

plamut commented Jun 1, 2021

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

@busunkim96
Copy link
Contributor

@plamut I looks like there is no CLA for [email protected]. @tseaver's CLA lists [email protected].

I think if you edit the Co-authored-by: Tres Seaver <[email protected]> in the commit body the CLA bot will be appeased. Please holler if that doesn't work.

@google-cla
Copy link

google-cla bot commented Jun 2, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jun 2, 2021
@plamut
Copy link
Contributor Author

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.

@plamut plamut added the automerge Merge the pull request once unit tests and other checks pass. label Jun 2, 2021
@plamut
Copy link
Contributor Author

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.

@plamut plamut merged commit d792ce0 into googleapis:master Jun 2, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 2, 2021
@plamut plamut deleted the iss-661 branch June 2, 2021 07:17
@plamut
Copy link
Contributor Author

plamut commented Jun 2, 2021

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

@simonvanderveldt
Copy link

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

@tseaver
Copy link
Contributor

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 [email protected] email address. The CLAbot seems flaky / fragile for commits which are merges of through-the-web suggestions.

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 googleapis/python-bigquery API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add support for ParquetOptions for load/external tables
6 participants