fix: updating testing, rest-only generation, & minor bug-fixes #716
Conversation
google-cla
bot
added
the
cla: yes
yon-mg
changed the title
| @@ -10,14 +10,19 @@ import math | |||
| import pytest | |||
| from proto.marshal.rules.dates import DurationRule, TimestampRule | |||
|
|
|||
| from requests import Response | |||
There was a problem hiding this comment.
Should this be under "if" statements?
| @pytest.mark.parametrize("client_class", [{{ service.client_name }}, {{ service.async_client_name }}]) | ||
| @pytest.mark.parametrize("client_class", [ | ||
| {{ service.client_name }}, | ||
| {%- if 'grpc' in opts.transport %} |
There was a problem hiding this comment.
This construct is used everywhere in the template. Since it is so commonly used, please consider simplifying it by introducing some helper method, such that it look something like:
{%- if grpc %}
stuff
{%- endif %}
Codecov Report
@@ Coverage Diff @@
## master #716 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 26 27 +1
Lines 1608 1619 +11
Branches 328 328
=========================================
+ Hits 1608 1619 +11
Continue to review full report at Codecov.
|
gapic/utils/checks.py
Outdated
| import re | ||
|
|
||
|
|
||
| def is_str(expr: str) -> bool: |
There was a problem hiding this comment.
If this file is used only for testing, please put it where the testing code is. Also please make sure that we don't do anything like this (guessting field type by its value, and using regexps for it), because it is not reliable enough and is quite slow (the regexp part).
|
@software-dov PTAL. This is the last PR which blocks us from generating the first Pyghon Alpha client for GCE! |
gapic/utils/checks.py
Outdated
| @@ -0,0 +1,42 @@ | |||
| # Copyright 2018 Google LLC | |||
There was a problem hiding this comment.
Please use field.type instead field.mock_value in your test templates. By doing that you may avoid guessing work here, as we discussed over GVC. Also, please update copyright to 2020 here and in all your new files.
| query_params = ['{k}={v}'.format(k=k, v=v) for k, v in query_params.items() if v] | ||
| url += '?{}'.format('&'.join(query_params)).replace(' ', '+') |
There was a problem hiding this comment.
It might be easier to use urllib.parse.urlencode here (It will create the k=v structure and glue the pairs together with & characters).
https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlencode
https://docs.python.org/3/library/urllib.request.html#urllib-examples
There was a problem hiding this comment.
Not sure how I missed this. Thanks!
There was a problem hiding this comment.
To clarify, I'll leave this out for now so I don't accidentally break anything but I'll update in the subsequent PR
yon-mg
changed the title
Co-authored-by: Bu Sun Kim <[email protected]>
| {% endif %} | ||
|
|
||
| {% if "next_page_token" in method.output.fields.values()|map(attribute='name') and not method.paged_result_field %} | ||
| {# Cheeser assertion to force code coverage for bad paginated methods #} |
There was a problem hiding this comment.
What does "cheeser" mean in this context?
| assert {i.name for i in cgr.file} == { | ||
| "foo/some_service/transports/river.py", | ||
| "foo/some_service/transports/car.py", | ||
| "foo/some_service/transports/__init__.py", | ||
| "foo/some_service/transports/base.py", | ||
| # Only generate async client with grpc transport |
There was a problem hiding this comment.
nit: This comment looks as thought it applies to the next line, but is a bit confusing. Suggest moving it below the next line and tweaking it to be something like: # no async_client.py, as that's only for grpc transport
Updating unit tests on generated client rest transport. Most changes made to unit tests are with compute api in mind and are not yet comprehensive enough for showcase.
Fixed issues with rest-only transport generation. Again, changes made with compute api in mind especially since compute api doesn't have grpc endpoint.
Fixed bug with attribute keyword conflicts.
The goal with this PR is to be able to generate a minimum usable GCE client library. Since rest transport is still under work, it is not generated by default and must be set with the transport flag
--python_gapic_opt="transport=grpc+rest".