FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: if JsonObject serialized to None then return `null_value` instead of `string_value` by o-aleks · Pull Request #771 · googleapis/python-spanner · GitHub

This repository was archived by the owner on Jun 8, 2026. It is now read-only.
/ python-spanner Public archive

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 commented Jul 15, 2022

Copy link
Copy Markdown
Contributor

No description provided.

o-aleks requested review from a team July 15, 2022 14:58

google-cla Bot commented Jul 15, 2022

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-label Bot added size: xs Pull request size is extra small. api: spanner Issues related to the googleapis/python-spanner API. labels Jul 15, 2022
asthamohta added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 19, 2022
yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 19, 2022
parthea 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-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 12, 2022
yoshi-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

Copy link
Copy Markdown
Contributor

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

o-aleks commented Aug 22, 2022
edited
Loading

Copy link
Copy Markdown
Contributor Author

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.

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

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 commented Aug 24, 2022

Copy link
Copy Markdown
Contributor Author

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-label Bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Aug 24, 2022
asthamohta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2022
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2022
IlyaFaer merged commit 82170b5 into googleapis:main Aug 25, 2022

Copy link
Copy Markdown

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

gcf-merge-on-green Bot 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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spanner Issues related to the googleapis/python-spanner API. size: s Pull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL