chore: fix CI invocations to use Python 3.9 (#1148)

Author: software-dov

packages/gapic-generator/.github/workflows/tests.yaml

@@ -18,10 +18,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
           cache: 'pip'
       - name: Install nox.
         run: python -m pip install nox
@@ -31,10 +31,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
           cache: 'pip'
       - name: Install nox.
         run: python -m pip install nox
@@ -47,10 +47,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
           cache: 'pip'
       - name: Install system dependencies.
         run: |
@@ -90,10 +90,10 @@ jobs:
         run: |
           sudo mkdir -p /tmp/workspace/tests/cert/
           sudo chown -R ${USER} /tmp/workspace/
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
           cache: 'pip'
       - name: Copy mtls files
         run: cp tests/cert/mtls.* /tmp/workspace/tests/cert/
@@ -172,10 +172,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
           cache: 'pip'
       - name: Install system dependencies.
         run: |
@@ -202,10 +202,10 @@ jobs:
         variant: ['', _alternative_templates]
     steps:
       - uses: actions/checkout@v2
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
           cache: 'pip'
       - name: Install system dependencies.
         run: |
@@ -229,10 +229,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
           cache: 'pip'
       - name: Install system dependencies.
         run: |
@@ -320,10 +320,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - name: Set up Python 3.8
+      - name: Set up Python 3.9
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.9
           cache: 'pip'
       - name: Install autopep8
         run: |

packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/transports/rest.py.j2

@@ -19,7 +19,8 @@ from google.protobuf import json_format
 {% endif %}
 from requests import __version__ as requests_version
 import dataclasses
-from typing import Callable, Dict, Optional, Sequence, Tuple, Union
+import re
+from typing import Callable, Dict, List, Optional, Sequence, Tuple, Union
 import warnings
 
 try:
@@ -65,7 +66,7 @@ class {{ service.name }}RestInterceptor:
 
     .. code-block:
         class MyCustom{{ service.name }}Interceptor({{ service.name }}RestInterceptor):
-{% for _, method in service.methods|dictsort if not (method.server_streaming or method.client_streaming) %}
+    {% for _, method in service.methods|dictsort if not (method.server_streaming or method.client_streaming) %}
             def pre_{{ method.name|snake_case }}(request, metadata):
                 logging.log(f"Received request: {request}")
                 return request, metadata
@@ -81,7 +82,7 @@ class {{ service.name }}RestInterceptor:
 
 
     """
-    {% for method in service.methods.values()|sort(attribute="name") if not(method.server_streaming or method.client_streaming) %}
+    {% for method in service.methods.values()|sort(attribute="name") if not (method.server_streaming or method.client_streaming) %}
     def pre_{{ method.name|snake_case }}(self, request: {{method.input.ident}}, metadata: Sequence[Tuple[str, str]]) -> Tuple[{{method.input.ident}}, Sequence[Tuple[str, str]]]:
         """Pre-rpc interceptor for {{ method.name|snake_case }}
 
@@ -175,6 +176,14 @@ class {{service.name}}RestTransport({{service.name}}Transport):
         # TODO(yon-mg): resolve other ctor params i.e. scopes, quota, etc.
         # TODO: When custom host (api_endpoint) is set, `scopes` must *also* be set on the
         # credentials object
+        maybe_url_match = re.match("^(?P<scheme>http(?:s)?://)?(?P<host>.*)$", host)
+        if maybe_url_match is None:
+            raise ValueError(f"Unexpected hostname structure: {host}")  # pragma: NO COVER
+
+        url_match_items = maybe_url_match.groupdict()
+
+        host = f"{url_scheme}://{host}" if not url_match_items["scheme"] else host
+
         super().__init__(
             host=host,
             credentials=credentials,
@@ -184,7 +193,7 @@ class {{service.name}}RestTransport({{service.name}}Transport):
         self._session = AuthorizedSession(
             self._credentials, default_host=self.DEFAULT_HOST)
         {% if service.has_lro %}
-        self._operations_client = None
+        self._operations_client: Optional[operations_v1.AbstractOperationsClient] = None
         {% endif %}
         if client_cert_source_for_mtls:
             self._session.configure_mtls_channel(client_cert_source_for_mtls)
@@ -202,7 +211,7 @@ class {{service.name}}RestTransport({{service.name}}Transport):
         """
         # Only create a new client if we do not already have one.
         if self._operations_client is None:
-            http_options = {
+            http_options: Dict[str, List[Dict[str, str]]] = {
             {% for selector, rules in api.http_options.items() %}
             {% if selector.startswith('google.longrunning.Operations') %}
                 '{{ selector }}': [
@@ -238,9 +247,10 @@ class {{service.name}}RestTransport({{service.name}}Transport):
         def __hash__(self):
             return hash("{{method.name}}")
 
+
         {% if not (method.server_streaming or method.client_streaming) %}
         {% if method.input.required_fields %}
-        __REQUIRED_FIELDS_DEFAULT_VALUES =  {
+        __REQUIRED_FIELDS_DEFAULT_VALUES: Dict[str, str] =  {
         {% for req_field in method.input.required_fields if req_field.is_primitive and req_field.name in method.query_params %}
             "{{ req_field.name | camel_case }}" : {% if req_field.field_pb.type == 9 %}"{{req_field.field_pb.default_value }}"{% else %}{{ req_field.type.python_type(req_field.field_pb.default_value or 0) }}{% endif %},{# default is str #}
         {% endfor %}
@@ -258,7 +268,7 @@ class {{service.name}}RestTransport({{service.name}}Transport):
                 retry: OptionalRetry=gapic_v1.method.DEFAULT,
                 timeout: float=None,
                 metadata: Sequence[Tuple[str, str]]=(),
-                ) -> {{method.output.ident}}:
+                ){% if not method.void %} -> {{method.output.ident}}{% endif %}:
     {% if method.http_options and not (method.server_streaming or method.client_streaming) %}
             r"""Call the {{- ' ' -}}
             {{ (method.name|snake_case).replace('_',' ')|wrap(
@@ -282,7 +292,7 @@ class {{service.name}}RestTransport({{service.name}}Transport):
             {% endif %}
             """
 
-            http_options = [
+            http_options: List[Dict[str, str]] = [
             {%- for rule in method.http_options %}{
                 'method': '{{ rule.method }}',
                 'uri': '{{ rule.uri }}',
@@ -330,8 +340,7 @@ class {{service.name}}RestTransport({{service.name}}Transport):
             headers = dict(metadata)
             headers['Content-Type'] = 'application/json'
             response = getattr(self._session, method)(
-                # Replace with proper schema configuration (http/https) logic
-                "https://{host}{uri}".format(host=self._host, uri=uri),
+                "{host}{uri}".format(host=self._host, uri=uri),
                 timeout=timeout,
                 headers=headers,
                 params=rest_helpers.flatten_query_params(query_params),
@@ -344,6 +353,7 @@ class {{service.name}}RestTransport({{service.name}}Transport):
             # subclass.
             if response.status_code >= 400:
                 raise core_exceptions.from_http_response(response)
+
             {% if not method.void %}
             # Return the response
             {% if method.lro %}
@@ -357,6 +367,7 @@ class {{service.name}}RestTransport({{service.name}}Transport):
             {% endif %}{# method.lro #}
             resp = self._interceptor.post_{{ method.name|snake_case }}(resp)
             return resp
+
             {% endif %}{# method.void #}
      {% else %}{# method.http_options and not (method.server_streaming or method.client_streaming) #}
             {% if not method.http_options %}
@@ -384,7 +395,9 @@ class {{service.name}}RestTransport({{service.name}}Transport):
         if not stub:
             stub = self._STUBS["{{method.name | snake_case}}"] = self._{{method.name}}(self._session, self._host, self._interceptor)
 
-        return stub
+        # The return type is fine, but mypy isn't sophisticated enough to determine what's going on here.
+        # In C++ this would require a dynamic_cast
+        return stub # type: ignore
 
     {% endfor %}
 

packages/gapic-generator/gapic/ads-templates/docs/conf.py.j2

@@ -28,7 +28,7 @@ __version__ = "0.1.0"
 # -- General configuration ------------------------------------------------
 
 # If your documentation needs a minimal Sphinx version, state it here.
-needs_sphinx = "1.6.3"
+needs_sphinx = "4.0.1"
 
 # Add any Sphinx extension module names here, as strings. They can be
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom

packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2

@@ -1251,7 +1251,7 @@ def test_{{ method.name|snake_case }}_rest_flattened():
         assert len(req.mock_calls) == 1
         _, args, _ = req.mock_calls[0]
         {% with uri = method.http_options[0].uri %}
-        assert path_template.validate("https://%s{{ uri }}" % client.transport._host, args[1])
+        assert path_template.validate("%s{{ uri }}" % client.transport._host, args[1])
         {% endwith %}
         {# TODO(kbandes) - reverse-transcode request args to check all request fields #}
 

packages/gapic-generator/gapic/schema/api.py

@@ -628,15 +628,25 @@ def proto(self) -> Proto:
 
     @cached_property
     def api_enums(self) -> Mapping[str, wrappers.EnumType]:
-        return collections.ChainMap({}, self.proto_enums,
-                                    *[p.all_enums for p in self.prior_protos.values()],
-                                    )
+        return collections.ChainMap(
+            {},
+            self.proto_enums,
+            # This is actually fine from a typing perspective:
+            # we're agglutinating all the prior protos' enums, which are
+            # stored in maps. This is just a convenient way to expand it out.
+            *[p.all_enums for p in self.prior_protos.values()],  # type: ignore
+        )
 
     @cached_property
     def api_messages(self) -> Mapping[str, wrappers.MessageType]:
-        return collections.ChainMap({}, self.proto_messages,
-                                    *[p.all_messages for p in self.prior_protos.values()],
-                                    )
+        return collections.ChainMap(
+            {},
+            self.proto_messages,
+            # This is actually fine from a typing perspective:
+            # we're agglutinating all the prior protos' enums, which are
+            # stored in maps. This is just a convenient way to expand it out.
+            *[p.all_messages for p in self.prior_protos.values()],  # type: ignore
+        )
 
     def _load_children(self,
                        children: Sequence, loader: Callable, *,

packages/gapic-generator/gapic/schema/wrappers.py

@@ -770,16 +770,16 @@ class HttpRule:
     uri: str
     body: Optional[str]
 
-    def path_fields(self, method: "~.Method") -> List[Tuple[Field, str, str]]:
+    def path_fields(self, method: "Method") -> List[Tuple[Field, str, str]]:
         """return list of (name, template) tuples extracted from uri."""
         input = method.input
         return [(input.get_field(*match.group("name").split(".")), match.group("name"), match.group("template"))
                 for match in path_template._VARIABLE_RE.finditer(self.uri)]
 
-    def sample_request(self, method: "~.Method") -> str:
+    def sample_request(self, method: "Method") -> Dict[str, Any]:
         """return json dict for sample request matching the uri template."""
 
-        def sample_from_path_fields(paths: List[Tuple["wrappers.Field", str, str]]) -> Dict[Any, Any]:
+        def sample_from_path_fields(paths: List[Tuple[Field, str, str]]) -> Dict[str, Any]:
             """Construct a dict for a sample request object from a list of fields
                and template patterns.
 
@@ -1040,7 +1040,11 @@ def query_params(self) -> Set[str]:
         params = set(self.path_params)
         body = self.http_opt.get('body')
         if body:
-            params.add(body)
+            if body == "*":
+                # The entire request is the REST body.
+                return set()
+            else:
+                params.add(body)
 
         return set(self.input.fields) - params