Prefer absolute import paths for google.api_core

[busunkim96]

To prevent clashes with API types we should prefer absolute imports for api_core imports.

For example, if an API has a type operation the generated code will be invalid:

from google.api_core import operation
from google.cloud.bar_v1 import operation

@dizcology reported this, but I'm having a little trouble figuring out where this is in the templates. Please ignore until I do some more investigation.

[software-dov]

I don't think this is a serious problem for a few reasons.
The generated surface uses operations via the following imports:

from google.api_core import operations_v1  # type: ignore
from google.longrunning import operations_pb2 as operations  # type: ignore

The wrapped types themselves are CamelCase, so that shouldn't generate a collision.
Last, and kind of catch-all, any name that might clash should probably be flagged during API review.

[busunkim96]

@dizcology Would you mind sending a tar of the library over email? Did you generate with the latest version of the generator or by using the Docker image?

[busunkim96]

@dizcology sent me a tar of the library over email.

The problematic API has a type called 'operation' and uses LROs

import "google/cloud/bar/master/operation.proto";
import "google/longrunning/operations.proto";

I think this is what results in there being two operations in client.py.

(google.api_core.operation is a wrapper around google.longrunning.operations_pb2 and is different from google.api_core.operation_v1, the client for working with operations)

from collections import OrderedDict
from typing import Dict, Sequence, Tuple, Type, Union
import pkg_resources

import google.api_core.client_options as ClientOptions # type: ignore
from google.api_core import exceptions                 # type: ignore
from google.api_core import gapic_v1                   # type: ignore
from google.api_core import retry as retries           # type: ignore
from google.auth import credentials                    # type: ignore
from google.oauth2 import service_account              # type: ignore

from google.api_core import operation  # <--- LRO
from google.cloud.bar_v1alpha1.services.dataset_service import pagers
from google.cloud.bar_v1alpha1.types import annotation
from google.cloud.bar_v1alpha1.types import annotation_spec
from google.cloud.bar_v1alpha1.types import data_item
from google.cloud.bar_v1alpha1.types import dataset
from google.cloud.bar_v1alpha1.types import dataset_service
from google.cloud.bar_v1alpha1.types import operation  # <--- API type
from google.protobuf import empty_pb2 as empty  # type: ignore
from google.protobuf import field_mask_pb2 as field_mask  # type: ignore

from .transports.base import DatasetServiceTransport
from .transports.grpc import DatasetServiceGrpcTransport

FWIW the monolith seems to use full import paths for things in api_core

https://github.com/googleapis/python-automl/blob/9c70f1a59db59c20c59efed0a165bdd3b18842fe/google/cloud/automl_v1/gapic/auto_ml_client.py#L19-L51

import functools
import pkg_resources
import warnings

from google.oauth2 import service_account
import google.api_core.client_options
import google.api_core.gapic_v1.client_info
import google.api_core.gapic_v1.config
import google.api_core.gapic_v1.method
import google.api_core.gapic_v1.routing_header
import google.api_core.grpc_helpers
import google.api_core.operation
import google.api_core.operations_v1
import google.api_core.page_iterator
import google.api_core.path_template
import google.api_core.protobuf_helpers
import grpc

from google.cloud.automl_v1.gapic import auto_ml_client_config
from google.cloud.automl_v1.gapic import enums
from google.cloud.automl_v1.gapic.transports import auto_ml_grpc_transport
from google.cloud.automl_v1.proto import annotation_spec_pb2
from google.cloud.automl_v1.proto import dataset_pb2
from google.cloud.automl_v1.proto import image_pb2
from google.cloud.automl_v1.proto import io_pb2
from google.cloud.automl_v1.proto import model_evaluation_pb2
from google.cloud.automl_v1.proto import model_pb2
from google.cloud.automl_v1.proto import operations_pb2 as proto_operations_pb2
from google.cloud.automl_v1.proto import service_pb2
from google.cloud.automl_v1.proto import service_pb2_grpc
from google.longrunning import operations_pb2 as longrunning_operations_pb2
from google.protobuf import empty_pb2
from google.protobuf import field_mask_pb2

[software-dov]

My first inclination is to kick the API back to API review and make the owners choose a different moniker.

Ahh....

Now that that's out of the way, assuming it's a non-starter, this should be a fairly simple fix but low priority fix. AutoML includes generated samples, which pushes it down the priority queue of APIs to try to convert.

[busunkim96]

My first inclination is to kick the API back to API review and make the owners choose a different moniker.

@software-dov Fair enough, I think @dizcology is doing some exploratory work with this API and it isn't quite ready to be published.

I haven't tried generating AutoML so I'm not sure if it will be bitten by the same problem.

I'll leave this as a FR for now and only change the priority if it becomes a blocker for producing a library.

[software-dov]

Oh, my mistake: from the in-code samples I was under the (mistaken) impression that the API in question was AutoML

[dizcology]

It seems unlikely at this point to revisit this on the APIs. I rather hope some changes could be made on the generator or code template to ensure name conflicts like this would not happen. Anything I can do to help?

[software-dov]

Why is it unlikely to revisit this on the APIs? This isn't a hill I'm willing to die on, but it seems simpler at this stage to change the API to not use the operation moniker. Let this comment indicate pushback on my part.

[busunkim96]

'operation' doesn't strike me as being a terribly strange name for a type in an API, and it looks like AutoML has already done it. https://github.com/googleapis/googleapis/blob/master/google/cloud/automl/v1/operations.proto.

I tried to check if AutoML runs into the same issue but I'm unable to generate it -- I think it's not annotated?

Yuhan and I would be willing to contribute a PR if you can point us in the right direction. 😄

[dizcology]

I think it is unlikely because "operation" describes the concept properly, there is the similar precedence of AutoML's operations, and it is currently at a late stage of the design phase.

Generally I think the API producers' expectation of the library generator is that it should be fully agnostic of the names in the API. (By the way I am not sure if similar problems could occur for other languages.)

[software-dov]

I think this has been dealt with by #615 . Will reopen if that's not the case.