This is what entity_pb looks like after line 150 in test_helpers.py after my first review change

key {
  partition_id {
    project_id: "PROJECT"
  }
  path {
    kind: "KIND"
    id: 1234
  }
}
properties {
  key: "baz"
  value {
    array_value {
    }
  }
}

@lukesneeringer @tseaver Are we concerned about something else breaking if we merge this in?

From what I understand of the code, the bug was only a problem if value has [] in it, which is fine.
But the set was checking if whether all of the values had the same exclusion, it was comparing it to length 1, which missed the empty array which is always True.

The other solution would be to check to see if len(set) < 1 instead of != 1 and then make sure we catch the pop KeyError, but that's similar to just checking to see if the array has something in it.

It seems that the only case that the merged code would be different is that value has an [] in it, in which something else would occur. However, since the code only appends to exclude_from_indexes. Every other case of array_value would remain the same, and an empty array would have no excluded indexes to worry about.

Should we do a

try:
  merge
except:
  fix

here or is there still too much risk concern?

Or maybe Luke's version was better, since maybe the empty list may get excluded also? I originally thought we only needed to check the list, but on second thought, the array_value itself may have been excluded.

Changed to Luke's version

I'll try again later.

Superseded by #4915.