Utility methods to write to dictionaries or protobuf Messages #183
Conversation
|
Assigning review to both @geigerj and @jonparrott. This is actually pretty urgent because of the partial Veneer timeline for Vision, so I am merging it once either of you gives it a thumbs up. |
| raise KeyError. | ||
|
|
||
| Args: | ||
| pb_or_dict: An object which may be either: |
There was a problem hiding this comment.
What is the motivation for supporting dict?
There was a problem hiding this comment.
You can basically always send a dictionary to any GAPIC function that requires a protobuf.message.Message subclass. This is huge for usability, because it is a lot more palatable to tell a user to issue:
request = {
'image': {'source': {'image_uri': 'http://bogus.bogus/img.jpg'}},
'features': ['FACE_DETECTION'],
}...than it is to ask them to send the equivalent protobuf messages (the example above would require four imports).
There was a problem hiding this comment.
Is that the case right now? I didn't realize that was possible currently.
There was a problem hiding this comment.
Yep! It has been possible at least since I joined Google. :-)
| key (str): The key to retrieve from the object in question. | ||
| default (Any): If the key is not present on the object, and a default | ||
| is set, returns that default instead. A type-appropriate falsy | ||
| default is generally recommended, as protobuf messages almost |
There was a problem hiding this comment.
as protobuf messages almost always have default values
To be more explicit, primitive-type protobuf fields always have default values. In contrast, message-type fields never do.
There was a problem hiding this comment.
Incorrect:
>>> from google.cloud.proto.vision.v1 import image_annotator_pb2
>>> request = image_annotator_pb2.AnnotateImageRequest()
>>> request.features
<google.protobuf.pyext._message.RepeatedCompositeContainer object at 0x7f862b64b2b8>
>>> bool(request.features)
False
>>> image_annotator_pb2.AnnotateImageRequest.mro()
[<class 'google.cloud.proto.vision.v1.image_annotator_pb2.AnnotateImageRequest'>, <class 'google.protobu
f.pyext._message.CMessage'>, <class 'google.protobuf.message.Message'>, <class 'object'>]There was a problem hiding this comment.
Sorry, message-type fields are not the same as repeated message-type fields. The latter defaults to an empty list.
I just meant to point out that proto3 has a distinction between default and unset values, and the semantics are predictable based on field type (https://developers.google.com/protocol-buffers/docs/proto3#default), whereas this comment was more hazy.
google/gax/utils/protobuf.py
Outdated
| """ | ||
| subkey = None | ||
| if separator in key: | ||
| subkey = separator.join(key.split(separator)[1:]) |
There was a problem hiding this comment.
optional nit: seems simpler just to use index rather than twice splitting the string (and once re-joining most of it)
| set(pb_or_dict, key, value) | ||
|
|
||
|
|
||
| def _resolve_subkeys(key, separator='.'): |
There was a problem hiding this comment.
You don't appear to configure separator anywhere, so why provide it as a kwarg?
There was a problem hiding this comment.
Easy future-proofing that costs me nothing.
You hit the nail on the head. :-) This is for use in partial GAPICs generally. A common partial GAPIC use case is likely going to be to take a portion of the required request and fill other information in. In my Vision helper, the single-image annotation will ask for all features if you leave off that input: protobuf.setdefault(request, 'features', self._get_all_features()) |
google/gax/utils/protobuf.py
Outdated
| __all__ = ('get', 'set', 'setdefault') | ||
|
|
||
|
|
||
| SENTINEL = object() |
google/gax/utils/protobuf.py
Outdated
| if separator in key: | ||
| subkey = separator.join(key.split(separator)[1:]) | ||
| key = key.split(separator)[0] | ||
| return (key, subkey) |
There was a problem hiding this comment.
nit: no need for ( and ) for tuple return types.
|
Merging once CI goes green. |
This PR adds a set of utility methods for reading or writing values from objects that might be dictionaries and might be subclasses of
google.protobuf.message.Message.Needed for partial Veneers.