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

Allowing heterogeneous meanings for datastore list properties. #1758

Merged
merged 1 commit into from
Apr 29, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Apr 27, 2016

Fixes #1649.

@pcostell PTAL

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 27, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Apr 27, 2016

Running my repro snippet after this change verifies that proto -> Entity -> proto doesn't drop any information on the floor.

# python pb_repro.py
e._meanings:
{u'blocks.value': ([None, 15, 15, None], [u'oops', u'foo', u'bar', u'forgot'])}
pb_new == pb:
True

return

# For lists, we set meaning on each sub-element.
if is_list:

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor Author

dhermes commented Apr 29, 2016

@tseaver PTAL.

@pcostell Is this good from your end?

@pcostell
Copy link
Contributor

Yup, LGTM

On Fri, Apr 29, 2016 at 10:02 AM Danny Hermes notifications@github.com
wrote:

@tseaver https://github.com/tseaver PTAL.

@pcostell https://github.com/pcostell Is this good from your end?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1758 (comment)

@tseaver
Copy link
Contributor

tseaver commented Apr 29, 2016

Given that we've decided to incorporate the notion of heterogenous meanings at all (a feature which seems quite dubious to me), the implementation looks good to merge.

@dhermes dhermes merged commit c3331eb into googleapis:master Apr 29, 2016
@dhermes dhermes deleted the fix-1649 branch April 29, 2016 18:06
@dhermes
Copy link
Contributor Author

dhermes commented Apr 29, 2016

@tseaver Yes I feel like the meanings code is a very steep uphill climb.

@tseaver tseaver added the api: datastore Issues related to the Datastore API. label May 16, 2016
@tseaver tseaver mentioned this pull request May 16, 2016
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.

4 participants