fix: Adjust Field Names in URI Templates #1041
Conversation
google-cla
bot
added
the
cla: yes
kbandes
changed the title
kbandes
changed the title
| @@ -452,6 +452,19 @@ def test_method_http_options_additional_bindings(): | |||
| }] | |||
|
|
|||
|
|
|||
| def test_method_http_options_reserved_name_in_url(): | |||
There was a problem hiding this comment.
Can we also add the short template format ({license}) here too (and ideally some other reserverd name besides license).
In general, please treat the short format as a first-class citizen for all grpc transcodding work, because this is the only format used in compute, which is the most importantlibrary relying on rest format.
There was a problem hiding this comment.
I have that form in the unit test test_uri_conv.py. I can add other names.
There was a problem hiding this comment.
Actually, the test there already includes reversed as well as license:
def test_convert_uri_fieldname():
uri = "abc/*/license/{license}/{xyz.reversed=reversed/*}"
expected_uri = "abc/*/license/{license_}/{xyz.reversed_=reversed/*}"
assert utils.convert_uri_fieldnames(uri) == expected_uri
There was a problem hiding this comment.
Why is license a reserved keyword? I think this should be fixed.
|
Validated that these changes fix the compute problem. |
|
@busunkim96, @software-dov I pushed this PR to unblock compute work. Please review it and if you have any requested chagnes, I'm happy to push them with a subsequent PR. Thanks! |
|
|
||
| last = 0 | ||
| pieces = [] | ||
| for match in path_template._VARIABLE_RE.finditer(uri): |
There was a problem hiding this comment.
Can _VARIABLE_RE be made public in google-api-core? Otherwise someone might assume it's only used in google-api-core and make a change that breaks the generator.
Slightly lower priority since it's in the generator code and not in generated library code.
Message fields that conflict with python reserved names are modified with a trailing underscore. If such a field appears in a uri template in an http option on a method, it needs to get the corresponding adjustment.