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

Improve BQ test utils to support JSON in a more simple manner #22942

Merged
merged 7 commits into from
Sep 1, 2022

Conversation

pabloem
Copy link
Member

@pabloem pabloem commented Aug 29, 2022

BigQuery testing utilities (and some prod utilities as well) default to converting individual data values into strings when converting data to JSON-encoded TableRow objects. This makes some testing difficult, as data types can't be recovered properly.

This change ensures that JSON conversion preserves the data type when possible in JSON (boolean, int32, float, double) so that testing and other consumers of TableRow data can expect correct types.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI.

@pabloem
Copy link
Member Author

pabloem commented Aug 29, 2022

r: @Abacn

@github-actions
Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

@Abacn
Copy link
Contributor

Abacn commented Aug 29, 2022

Thanks for the fix. Could you please providing some context of the change, like what current issue we have using INT32?

@pabloem
Copy link
Member Author

pabloem commented Aug 30, 2022

thanks @Abacn - I've added an explanation on the top comment of the PR.

@pabloem
Copy link
Member Author

pabloem commented Aug 30, 2022

Run Java PostCommit

@pabloem
Copy link
Member Author

pabloem commented Aug 30, 2022

Run Java PostCommit

@pabloem pabloem closed this Aug 30, 2022
@pabloem pabloem reopened this Aug 30, 2022
@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

Retest this please

@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

feels like jenkins is targeting me personally : ) lol

@pabloem pabloem closed this Aug 31, 2022
@pabloem pabloem reopened this Aug 31, 2022
@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

Run Java PreCommit

@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

Run Java_Examples_Dataflow_Java11 PreCommit

@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

uggg these don't repro in my local env

@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

Run Java PostCommit

@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

Run Java_Examples_Dataflow PreCommit

@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

Run Java PreCommit

@pabloem
Copy link
Member Author

pabloem commented Aug 31, 2022

errors unrelated. Feel free to review @Abacn
Errors in previous run: https://ci-beam.apache.org/job/beam_PreCommit_Java_Commit/23775/

Copy link
Contributor

@Abacn Abacn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@pabloem
Copy link
Member Author

pabloem commented Sep 1, 2022

Run Java PreCommit

@pabloem pabloem merged commit 9c1c316 into apache:master Sep 1, 2022
@pabloem pabloem deleted the bqtestimprove branch September 1, 2022 15:30
dedocibula pushed a commit to dedocibula/beam that referenced this pull request Sep 15, 2022
…#22942)

* Improve BQ test utils to support JSON in a more simple manner

* Fix storage api path

* fix boolean tests

* fixup

* double is representable in JSON

* same

* Doubles can be represented in JSON as well
kkdoon pushed a commit to twitter-forks/beam that referenced this pull request Sep 29, 2022
…#22942)

* Improve BQ test utils to support JSON in a more simple manner

* Fix storage api path

* fix boolean tests

* fixup

* double is representable in JSON

* same

* Doubles can be represented in JSON as well
cushon pushed a commit to cushon/beam that referenced this pull request Oct 17, 2022
…#22942)

* Improve BQ test utils to support JSON in a more simple manner

* Fix storage api path

* fix boolean tests

* fixup

* double is representable in JSON

* same

* Doubles can be represented in JSON as well
Abacn pushed a commit to Abacn/beam that referenced this pull request Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants