Skip to content

Conversation

@udoprog
Copy link

@udoprog udoprog commented Nov 18, 2016

When the array_value is empty, the set also becomes empty which triggers the existing condition len(exclude_values) != 0.

This patch changes this condition to permit empty array values to pass through.

@googlebot
Copy link

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).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Nov 18, 2016
@udoprog
Copy link
Author

udoprog commented Nov 18, 2016

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot 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 Nov 18, 2016
@udoprog udoprog changed the title Support decoding empty arrays datastore: Support decoding empty arrays Nov 18, 2016
@daspecster daspecster added the api: datastore Issues related to the Datastore API. label Nov 18, 2016
@dhermes
Copy link
Contributor

dhermes commented Nov 18, 2016

@udoprog Can you give an example that currently fails, but would work under the changes in this PR?

@udoprog
Copy link
Author

udoprog commented Nov 18, 2016

The python client doesn't permit writing an empty array, so you'll have to do that using another client (we do it using Java).

Or, go into the cloud console and add an entity /test:test with an Array property that has the following value:

{
  "values": []
}

After this, run the following code:

from google.cloud import datastore

if __name__ == "__main__":
    client = datastore.Client('my-project-1')

    k = client.key('test', 'test')
    e = client.get(k)

    print(e)

Without this patch, you should see:

Traceback (most recent call last):
  File "example.py", line 9, in <module>
    e = client.get(k)
  File "/home/udoprog/.local/lib/python3.5/site-packages/google/cloud/datastore/client.py", line 256, in get
    deferred=deferred, transaction=transaction)
  File "/home/udoprog/.local/lib/python3.5/site-packages/google/cloud/datastore/client.py", line 316, in get_multi
    for entity_pb in entity_pbs]
  File "/home/udoprog/.local/lib/python3.5/site-packages/google/cloud/datastore/client.py", line 316, in <listcomp>
    for entity_pb in entity_pbs]
  File "/home/udoprog/.local/lib/python3.5/site-packages/google/cloud/datastore/helpers.py", line 142, in entity_from_protobuf
    raise ValueError('For an array_value, subvalues must either '
ValueError: For an array_value, subvalues must either all be indexed or all excluded from indexes.

@dhermes
Copy link
Contributor

dhermes commented Nov 18, 2016

Thanks @udoprog!

In order to actually merge this in, we'll need some unit tests. You can write them yourself, or I am happy to write them, since you've allowed maintainers to push commits to your PR branch.

@daspecster
Copy link
Contributor

@dhermes won't google CLA bot wig out if multiple people commit?

@dhermes
Copy link
Contributor

dhermes commented Nov 29, 2016

@daspecster It may, but we the humans can ignore it.

@udoprog
Copy link
Author

udoprog commented Nov 30, 2016

My apologies I can't take a more active part in this right now. Feel free to just see this as a bug report and fix things whatever way suits you best.

exclude_values = set(value_pb.exclude_from_indexes
for value_pb in value_pb.array_value.values)
if len(exclude_values) != 1:
if len(exclude_values) > 1:

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@lukesneeringer
Copy link
Contributor

See also #3152, where I attempted to fix this problem and hit another problem.

We likely do not need two issues for it, though, so I am closing this one.

parthea pushed a commit that referenced this pull request Sep 22, 2023
…oudPlatform/python-docs-samples#2755)

* Add auto-generated Logo Recognition Samples

* Add tests

Remove boilerplate
Update copyright date
Blacken
Remove unused imports
Shorten docstrings
Remove CLI
Set defaults in function definition

* Edit test video to 3 seconds from 35

Reduce quota usage to allow for parallel tests
Retain longer version only for tests that require it

* Fix bug

Co-authored-by: Noah Negrey <nnegrey@users.noreply.github.com>
Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: datastore Issues related to the Datastore API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants