Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

fix: if JsonObject serialized to None then return null_value instead of string_value - #771

Merged
IlyaFaer merged 8 commits into
googleapis:mainfrom
o-aleks:serialize_none_json
Aug 25, 2022
Merged

fix: if JsonObject serialized to None then return null_value instead of string_value#771
IlyaFaer merged 8 commits into
googleapis:mainfrom
o-aleks:serialize_none_json

Conversation

@o-aleks

Copy link
Copy Markdown
Contributor

No description provided.

@o-aleks
o-aleks requested review from a teamJuly 15, 2022 14:58
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@product-auto-labelproduct-auto-labelBot added size: xs Pull request size is extra small. api: spanner Issues related to the googleapis/python-spanner API. labels Jul 15, 2022
@asthamohtaasthamohta added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 19, 2022
@yoshi-kokoroyoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 19, 2022
@partheaparthea added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 12, 2022
@gcf-owl-botgcf-owl-botBot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 12, 2022
@yoshi-kokoroyoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 12, 2022
@asthamohta

Copy link
Copy Markdown
Contributor

hi @o-aleks, what is this PR for?

@o-aleks

o-aleks commented Aug 22, 2022

Copy link
Copy Markdown
ContributorAuthor

Hi @asthamohta,

There is a bug in _make_value_pb function.
If the value is an instance of JsonObject and it is None after serializing, then it should be translated to NULL_VALUE, but in the original code, it is a string.

My fix is:

def _make_value_pb(value):
...
if isinstance(value, JsonObject):
value = value.serialize()
if value is None:
return Value(null_value="NULL_VALUE")
else:
return Value(string_value=value)

Could you please check it and approve the PR?
Thank you.

@asthamohtaasthamohta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 24, 2022
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 24, 2022
@IlyaFaer

Copy link
Copy Markdown

Should be okay. There is an if statement upper in the method, which does the same for all the data types, but JsonObject is more complicated than other types, so it passes the condition (and we can't change this behavior for the class, because there is no magic method for is None statements).

A test should be added though. @o-aleks, will you find time to add a test? Or I can do it in a separate issue.

@o-aleks

Copy link
Copy Markdown
ContributorAuthor

A test should be added though. @o-aleks, will you find time to add a test? Or I can do it in a separate issue.
Oh, yes, of course, I'll add the test.

@product-auto-labelproduct-auto-labelBot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Aug 24, 2022
@asthamohtaasthamohta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2022
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2022
@IlyaFaer
IlyaFaer merged commit 82170b5 into googleapis:mainAug 25, 2022
@release-pleaserelease-pleaseBot mentioned this pull request Aug 25, 2022
@IlyaFaer

Copy link
Copy Markdown

Thanks, @o-aleks, looks good, and tests are passing fine. I'm merging it.

gcf-merge-on-greenBot pushed a commit that referenced this pull request Aug 30, 2022
🤖 I have created a release *beep* *boop*
---
## [3.20.0](v3.19.0...v3.20.0) (2022-08-30)
### Features
* Adds TypeAnnotationCode PG_JSONB ([#792](#792)) ([6a661d4](6a661d4))
### Bug Fixes
* if JsonObject serialized to None then return `null_value` instead of `string_value` ([#771](#771)) ([82170b5](82170b5))
---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
parthea pushed a commit to googleapis/google-cloud-python that referenced this pull request Mar 9, 2026
🤖 I have created a release *beep* *boop*
---
## [3.20.0](googleapis/python-spanner@v3.19.0...v3.20.0) (2022-08-30)
### Features
* Adds TypeAnnotationCode PG_JSONB ([#792](googleapis/python-spanner#792)) ([6a661d4](googleapis/python-spanner@6a661d4))
### Bug Fixes
* if JsonObject serialized to None then return `null_value` instead of `string_value` ([#771](googleapis/python-spanner#771)) ([82170b5](googleapis/python-spanner@82170b5))
---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spannerIssues related to the googleapis/python-spanner API.size: sPull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@o-aleks@asthamohta@IlyaFaer@parthea@yoshi-kokoro