Skip to content

Conversation

@crwilcox
Copy link
Contributor

@crwilcox crwilcox commented Oct 22, 2018

Fixes #6284.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 22, 2018
@tseaver
Copy link
Contributor

tseaver commented Oct 23, 2018

Needs test changes to match the new behavior.

Also, I'd like to hold off merging until @dhermes has a chance to chime in on my question in #6284.

@tseaver
Copy link
Contributor

tseaver commented Oct 24, 2018

@crwilcox Can you add a system test which exercises your change and verify that the empty list value is preserved in a round-trip? Something like:

# Inside 'tests/system/test_system.py', 'TestDatastoreSave'
    def test_w_empty_list(self):
        entity = self._get_post(id_or_name=(name or key_id))
        del entity['tags'][:]
        Config.CLIENT.put(entity)
        fetched = Config.CLIENT.get(entity.key)
        self.assertEqual(entity, fetched)

@crwilcox
Copy link
Contributor Author

@tseaver, sure. I had a unit test so was previously confused by the ask. Let me add to system though.

@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2018
@tseaver tseaver changed the title Fix #6284 by handling empty arrays in entity to pb Datastore: propagate empty arrays in entity values Oct 26, 2018
@tseaver tseaver added the api: datastore Issues related to the Datastore API. label Oct 26, 2018
@tseaver
Copy link
Contributor

tseaver commented Oct 26, 2018

Unrelated firestore failure: will be fixed ASAP via #6320.

@tseaver
Copy link
Contributor

tseaver commented Oct 26, 2018

I don't know what to do about the shouldn't-even-have-run WSS test failure: it borked trying to download a docker image.

@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2018
@crwilcox
Copy link
Contributor Author

@tseaver I don't believe these are related failures. Still I am going to rerun the tests.

@tseaver
Copy link
Contributor

tseaver commented Oct 26, 2018

@crwilcox Those failures clearly can't be related to this PR: it would be better if they didn't even run at all. That doesn't help for the WSS failure, as it borked before it could even check that websecurityscanner/ was in the diff, but the change detection in Kokoro is defective: we are running tests for unrelated APIs (e.g., firestore/ for the earlier failure).

@crwilcox crwilcox merged commit 0c13d62 into googleapis:master Oct 26, 2018
parthea pushed a commit that referenced this pull request Nov 24, 2025
* Fix #6284 by handling empty arrays in entity to pb
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