GAPIC-generated clients scribble on global DEFAULT_CLIENT_INFO

[tseaver]

tl;dr

The GAPIC-generated class constructors listed below scribble on the fallback google.api_core.gapic_v1.client_info.DEFAULT_CLIENT_INFO instance, rather than creating their own copy.

@crwilcox Can you please link in the related codegen issue?

Analysis

The pattern is that these client classes take an optional client_info parameter: if not passed, they use the DEFAULT_CLIENT_INFO instance. In either case, they then assign over its gapic_version attribute:

        if client_info is None:
            client_info = (
                google.api_core.gapic_v1.client_info.DEFAULT_CLIENT_INFO)
        client_info.gapic_version = _GAPIC_LIBRARY_VERSION
        self._client_info = client_info

I am certain that it is inappropriate to scribble on the global DEFAULT_CLIENT_INFO instance. Rather they should be creating a new google.api_core.gapic.client_info.ClientInfo instance. I
believe that if the user passes in a non-None client_info, these clients should not scribble on it. E.g.:

        if client_info is None:
            client_info = google.api_core.gapic_v1.client_info.ClientInfo(
                gapic_version = _GAPIC_LIBRARY_VERSION,
            )
        self._client_info = client_info

Offending Classes

• google.cloud.automl_v1beta1.gapic.auto_ml_client.AutoMlClient
• google.cloud.automl_v1beta1.gapic.prediction_service_client.PredictionServiceClient
• google.cloud.bigquery_datatransfer_v1.gapic.data_transfer_service_client.DataTransferServiceClient
• google.cloud.bigtable_admin_v2.gapic.bigtable_instance_admin_client.BigtableInstanceAdminClient
• google.cloud.bigtable_admin_v2.gapic.bigtable_table_admin_client.BigtableTableAdminClient
• google.cloud.bigtable_v2.gapic.bigtable_client.BigtableClient
• google.cloud.container_v1.gapic.cluster_manager_client.ClusterManagerClient
• google.cloud.dataproc_v1.gapic.cluster_controller_client.ClusterControllerClient
• google.cloud.dataproc_v1.gapic.job_controller_client.JobControllerClient
• google.cloud.datastore_v1.gapic.datastore_client.DatastoreClient
• google.cloud.dlp_v2.gapic.dlp_service_client.DlpServiceClient
• google.cloud.errorreporting_v1beta1.gapic.error_group_service_client.ErrorGroupServiceClient
• google.cloud.errorreporting_v1beta1.gapic.error_stats_service_client.ErrorStatsServiceClient
• google.cloud.errorreporting_v1beta1.gapic.report_errors_service_client.ReportErrorsServiceClient
• google.cloud.firestore_v1beta1.gapic.firestore_client.FirestoreClient
• google.cloud.iot_v1.gapic.device_manager_client.DeviceManagerClient
• google.cloud.kms_v1.gapic.key_management_service_client.KeyManagementServiceClient
• google.cloud.language_v1.gapic.language_service_client.LanguageServiceClient
• google.cloud.language_v1beta2.gapic.language_service_client.LanguageServiceClient
• google.cloud.logging_v2.gapic.config_service_v2_client.ConfigServiceV2Client
• google.cloud.logging_v2.gapic.logging_service_v2_client.LoggingServiceV2Client
• google.cloud.logging_v2.gapic.metrics_service_v2_client.MetricsServiceV2Client
• google.cloud.monitoring_v3.gapic.alert_policy_service_client.AlertPolicyServiceClient
• google.cloud.monitoring_v3.gapic.group_service_client.GroupServiceClient
• google.cloud.monitoring_v3.gapic.metric_service_client.MetricServiceClient
• google.cloud.monitoring_v3.gapic.notification_channel_service_client.NotificationChannelServiceClient
• google.cloud.monitoring_v3.gapic.uptime_check_service_client.UptimeCheckServiceClient
• google.cloud.oslogin_v1.gapic.os_login_service_client.OsLoginServiceClient
• google.cloud.pubsub_v1.gapic.publisher_client.PublisherClient
• google.cloud.pubsub_v1.gapic.subscriber_client.SubscriberClient
• google.cloud.redis_v1beta1.gapic.cloud_redis_client.CloudRedisClient
• google.cloud.spanner_admin_database_v1.gapic.database_admin_client.DatabaseAdminClient
• google.cloud.spanner_admin_instance_v1.gapic.instance_admin_client.InstanceAdminClient
• google.cloud.spanner_v1.gapic.spanner_client.SpannerClient
• google.cloud.speech_v1.gapic.speech_client.SpeechClient
• google.cloud.speech_v1p1beta1.gapic.speech_client.SpeechClient
• google.cloud.tasks_v2beta2.gapic.cloud_tasks_client.CloudTasksClient
• google.cloud.texttospeech_v1.gapic.text_to_speech_client.TextToSpeechClient
• google.cloud.texttospeech_v1beta1.gapic.text_to_speech_client.TextToSpeechClient
• google.cloud.trace_v1.gapic.trace_service_client.TraceServiceClient
• google.cloud.trace_v2.gapic.trace_service_client.TraceServiceClient
• google.cloud.videointelligence_v1.gapic.video_intelligence_service_client.VideoIntelligenceServiceClient
• google.cloud.videointelligence_v1beta1.gapic.video_intelligence_service_client.VideoIntelligenceServiceClient
• google.cloud.videointelligence_v1beta2.gapic.video_intelligence_service_client.VideoIntelligenceServiceClient
• google.cloud.videointelligence_v1p1beta1.gapic.video_intelligence_service_client.VideoIntelligenceServiceClient
• google.cloud.vision_v1.gapic.image_annotator_client.ImageAnnotatorClient
• google.cloud.vision_v1p1beta1.gapic.image_annotator_client.ImageAnnotatorClient
• google.cloud.vision_v1p2beta1.gapic.image_annotator_client.ImageAnnotatorClient
• google.cloud.vision_v1p3beta1.gapic.image_annotator_client.ImageAnnotatorClient
• google.cloud.vision_v1p3beta1.gapic.product_search_client.ProducSearchClient
• google.cloud.websecurityscanner_v1alpha.gapic.web_security_scanner_client.WebSecurityScannerClient

[tseaver]

Related gapic-generator bug.

[tseaver]

Once googleapis/gapic-generator#2289 is merged / released, we can re-run synth to pick up the fix.

[tseaver]

@theacodes Is there a reason to have this bug unassigned? @crwilcox was the one who reported the GAPIC generator bug.

[tseaver]

googleapis/gapic-generator#2289 looks stalled / dead: it has conflicts, and nobody is responding to a ping.

[tseaver]

W00t! the gapic-generator PR is merged as of today!

@theacodes I'm assuming we should see a bunch of autosynth PRs showing up from @dpebot as soon as the artman image is updated with the fix. Should we go ahead and close this issue, or maybe once they start showing up?

[theacodes]

Whichever you feel is most appropriate.

…

On Wed, Oct 31, 2018, 3:09 PM Tres Seaver ***@***.***> wrote: W00t! the gapic-generator PR is merged as of today! @theacodes <https://github.com/theacodes> I'm assuming we should see a bunch of autosynth PRs showing up from @dpebot <https://github.com/dpebot> as soon as the artman image is updated with the fix. Should we go ahead and close this issue, or maybe once they start showing up? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5710 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPUc6kerDBjFoYpWag3p6rsTgMrucqCks5uqh-ZgaJpZM4VosuQ> .

[tseaver]

Autosynth PRs with this fix have landed this morning.