googleads / google-ads-python

Google Ads API Client Library for Python
Apache License 2.0
519 stars 480 forks source link

querying video_quartile_25_rate causes google.protobuf.internal.well_known_types.Error: Fail to print FieldMask to Json string #30

Closed apurvis closed 5 years ago

apurvis commented 5 years ago

This issue arose trying to upgrade to package 0.6/API 0.7. Trying to query metrics.video_quartile_25_rate gives google.protobuf.internal.well_knowntypes.Error: Fail to print FieldMask to Json string: The character after a "" must be a lowercase letter in path name metrics.video_quartile_25_rate.

full stack trace:

File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/page_iterator.py", line 199, in _items_iter
   for page in self._page_iter(increment=False):
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/page_iterator.py", line 230, in _page_iter
   page = self._next_page()
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/page_iterator.py", line 512, in _next_page
   response = self._method(self._request)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/gapic_v1/method.py", line 139, in __call__
   return wrapped_func(*args, **kwargs)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/retry.py", line 260, in retry_wrapped_func
   on_error=on_error,
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/retry.py", line 177, in retry_target
   return target()
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/timeout.py", line 206, in func_with_timeout
   return func(*args, **kwargs)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/grpc_helpers.py", line 59, in error_remapped_callable
   return callable_(*args, **kwargs)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/grpc/_interceptor.py", line 201, in __call__
   credentials=credentials)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/grpc/_interceptor.py", line 226, in _with_call
   return call.result(), call
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/grpc/_interceptor.py", line 116, in result
   raise self._exception
google.protobuf.internal.well_known_types.Error: Fail to print FieldMask to Json string: The character after a "_" must be a lowercase letter in path name metrics.video_quartile_25_rate.
BenRKarl commented 5 years ago

Hi @apurvis - thanks for bringing this issue up! It looks like a problem with how the message is serialized to JSON. I'm looking into it now and will respond here once I have more information, and if there's a necessary code change I'll post a link to the PR here. As a temporary workaround - since the error is originating from the logging interceptor - if you set your logging level to ERROR the logging process will be skipped and these errors won't be triggered.

BenRKarl commented 5 years ago

@apurvis just an update on this - the issue is stemming from the protobuf library, specifically the ToJsonString method for FieldMasks. The method converts all strings in the FieldMask's paths list from snake case to camel case, but rejects digits that follow underscores (i.e. _2). See this line. I don't see why digits should prevent this conversion, so I've proposed a change to the protobuf team. Depending on how long that takes I may post a temporary fix to patch that method, but I would much prefer the change come from their library.

Apologies again for the inconvenience, but really do appreciate you pointing this out!

rubberviscous commented 5 years ago

@BenRKarl Is the ERROR error level still the preferred workaround? If so, what's the proper way to set this up? I tried passing a log dictionary to GoogleAdsClient class. But I'm still getting the Fail to print FieldMask to Json string exception. I've included my implementation below, is this the correct approach?

log_config = {
    'version': 1,
    'formatters': {
        'standard': {
            'format': '%(asctime)s [%(levelname)s] %(name)s: %(message)s'
        },
    },
    'handlers': {
        'default': {
            'level': 'ERROR',
            'formatter': 'standard',
            'class': 'logging.StreamHandler',
            'stream': 'ext://sys.stdout',  # Default is stderr
        },
    },
    'loggers': {
        'default': {
            'level': 'ERROR',
            'handlers': ['default']
        }
    }
}
client = GoogleAdsClient(
            credentials=credentials,
            developer_token=developer_token,
            login_customer_id=login_customer_id,
            logging_config=log_config
        )
BenRKarl commented 5 years ago

@rubberviscous what's the request you're making? If you're getting an API error then setting the level to ERROR may not be an effective workaround. If that's the case you may need to turn logging off entirely.

For background the protobuf utility we use here to convert protobuf message to JSON can not parse fields with a digit following an underscore, i.e. test_2_field. This line only fires when logs are generated. Setting to ERROR worked for @apurvis because his requests weren't returning API errors. Until I can post a fix to remove JSON serialization, I suggest disabling logging via the library and catching and logging the response directly in your code to diagnose the API error. Once you've stopped receiving API errors it should be safe to enabling logging and set back to ERROR in order to capture normal logs.

Sorry about the weird workaround. Hope the context makes sense, and that I can get this fixed soon.

BenRKarl commented 5 years ago

@rubberviscous linking this related issue here with a comment from @apurvis

https://github.com/googleads/google-ads-python/issues/75

rubberviscous commented 5 years ago

@BenRKarl Thank you for your response. I'm trying to make the following request:

client = (google.ads.google_ads.client.GoogleAdsClient.load_from_storage())
ga_service = client.get_service('GoogleAdsService', version='v1')
response = ga_service.search(account_id, query, page_size=page_size)

The query:

SELECT segments.date, segments.ad_network_type, metrics.video_quartile_25_rate, metrics.video_quartile_50_rate, metrics.video_quartile_75_rate, metrics.video_quartile_100_rate, metrics.active_view_viewability, metrics.average_cost, metrics.average_cpm, metrics.average_cpv, metrics.cost_micros, metrics.impressions, metrics.video_views, metrics.video_view_rate
FROM ad_group
WHERE segments.ad_network_type='YOUTUBE_WATCH'
LIMIT 10

I'm assuming the definition for API errors could mean a bad query sent to the API? If that's the case, my requests were working fine. It only started blowing up after I added the metrics.video_quartile_25_rate, etc.

Can you articulate on which library to disable logging for? The protobuf library or google-ads? I tried disabling via the standard logging module, but that didn't work: logging.getLogger("google.protobuf.json_format").setLevel(logging.WARNING)

I also see that the google.ads.google_ads.client.GoogleAdsClient has a logging_dict parameter. I also tried passing in a logging dict but that didn't work either. What's the correct way to disable logging here? Thanks in advance.

rubberviscous commented 5 years ago

~@BenRKarl I just discovered that there is a logging.disable method. I added logging.disable(logging.ERROR) to the script and it seems to work now. Is this what you were referring to?~

I was able to get the query working by adding this line logging.getLogger('google.ads.google_ads.client').setLevel(logging.ERROR)

Thanks!

BenRKarl commented 5 years ago

@rubberviscous that definitely works, or you could also disable logging via the configuration by commenting out or removing the lines related to logging.

I'm hoping to refactor the logging interceptor very soon so that it doesn't serialize to JSON, once that's finished I'll comment here to let you know.

BenRKarl commented 5 years ago

@rubberviscous @apurvis I was finally able to post a PR that removes json serialization of proto messages for logging. You can review it here: https://github.com/googleads/google-ads-python/pull/95

Please feel free to leave comments or questions. Once it's merged and published you can start requesting fields such as video_quartile_25_rate again.

Sorry for the delay on this one!

BenRKarl commented 5 years ago

Closing this as https://github.com/googleads/google-ads-python/pull/95 was merged and published.