Upgrade to support Google Ads v10

[anthonyjgraff]

Closes: #22111

Description

This PR updates the google-ads dependency to v15.1.1, which adds support for v10 of the Google Ads API.

The latest version of the Google Ads API supported by the current library is v8, which will be sunsetted on 2022-05-11 (https://developers.google.com/google-ads/api/docs/sunset-dates).

Previously, Airflow was limited to v14.0.0 of the library as more recent versions required v2.x of google-api-core. However, the google-ads library re-added support for v1.x of google-api-core in v15.1.1

Testing

This is my first Airflow PR - I tried to run the full Breeze test suite, but struggled as the new requirements conflict with the constraints files - any advice would be welcome!

However, I did build an image & run a few DAGs / operators that I run regularly, getting the expected result

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

• Pay attention to the quality of your code (flake8, mypy and type annotations). Our pre-commits will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@airflow.apache.org
  Slack: https://s.apache.org/airflow-slack

[anthonyjgraff]

Looks like the build is failing as colorlog is updating from v4.8 to v6.x, which has some backwards incompatible changes

Will pin colorlog to < 5 in this PR

[potiuk]

We should describae why we limited it - which tests failed - ideally what were the proble and what we need to fix it.

Or maybe we can fix it ? I doubt colorlog has some "really big" changes.

[anthonyjgraff]

It felt fixable when I first spotted it yesterday - the issues all seemed to be related to CustomTTYColoredFormatter but it didn't feel right to make changes to Airflow logging as part of this PR, as it's totally unrelated to Google Ads

It was actually mypy that threw up errors - the TTYColoredFormatter class we inherit from was merged into ColoredFormatter, and as a result, a few functions have changed names / signatures.

Can introduce a comment as part of this PR, and can look to upgrade it in a separate PR

[potiuk]

Separate PR is way better. Just comment in this one as we have a rule thall all upper-bounded limitations have to have comment why and what is needed to lift it.

https://github.com/apache/airflow/blob/main/README.md#approach-for-dependencies-for-airflow-core

Whenever we upper-bound such a dependency, we should always comment why we are doing it - i.e. we should have a good reason why dependency is upper-bound. And we should also mention what is the condition to remove the binding.

[potiuk]

Cool. I removed the empty requirements.txt added there by mistake :)

[anthonyjgraff]

Any idea what would have caused the above failure? It doesn't look like it's related to anything changed in this PR.

[potiuk]

Likely transient failure. You can always "rerun failed jobs" if it happens.

[boring-cyborg]

Awesome work, congrats on your first merged pull request!

[k1my0]

How can I use it? Is this PR merged normally?
@jedcunningham @anthonyjgraff @potiuk

[potiuk]

Yes. It's merged. You will have to wait unit Google Provider gets released (which is going to happen in the next few days as we are preparing for a new wave of those.

[k1my0]

Thanks!

[anthonyjgraff]

Sounds great. Just as a heads up, V8 of the API will stop working on Wednesday 11th - definitely makes sense to group the Google Provider changes together, but we'll be a bit stuck if a new version of the provider hasn't been released by then!

[SarbjitGahra]

Sounds great. Just as a heads up, V8 of the API will stop working on Wednesday 11th - definitely makes sense to group the Google Provider changes together, but we'll be a bit stuck if a new version of the provider hasn't been released by then!

Did you find any solution for this ? I am running into same issue. Unable to update to the latest version because of google provider requirement range being <14.0.1
Any help is appreciated?

[potiuk]

Sounds great. Just as a heads up, V8 of the API will stop working on Wednesday 11th - definitely makes sense to group the Google Provider changes together, but we'll be a bit stuck if a new version of the provider hasn't been released by then!

I am going to send it to voting during the weekend most likely. It will take min 72 hours, But if bugs are found and we cancel the RC it might take multiple of those 72 hrs. So I think if your business depends on it , you should work on a contingency plan. One of the options you have is taking the unreleased code of the providers and copy it to your dags folder and use it from there.

But I am not sure also if everyone is able to upgrade after that - the new providers will not work for those on 2.0 for example, but this is something that is on Google Ads team to drop the APIs, this is not something we can do anything about, if the ads team make the decision to switch off the APIs on Wed, then for sure they accepted the risk that some users will not be able to upgrade some of the clients - and they are the only ones you can appeal to if you badly need it to continue working.

[potiuk]

Just wanted you to give heads up @SarbjitGahra @anthonyjgraff that if your business continuity depends on the ads, this is not something our decisions on releasing or not releasing the provider are based. It's your thing to make sure you have contingency plan.

[jiamo]

Airflow == 2.2.4 Hi. after install pip install apache-airflow-providers-google==7.0.0rc1 and change v8 to v10

        service = GoogleAdsHook(
            gcp_conn_id=self.gcp_conn_id,
            google_ads_conn_id=self.google_ads_conn_id,
            api_version="v8")  # here change
        rows = service.search(client_ids=self.client_ids,
                              query=self.query, page_size=self.page_size)

I got error :

TypeError: Invalid constructor input for SearchGoogleAdsRequest: customer_id:

[anthonyjgraff]

@jiamo customer_id comes from the value passed to client_ids - do you know what value you're using for self.client_ids?

[jiamo]

self.client_ids was query from db . it is an array of customer_id. Same code works for v8

[jiamo]

The total trace

Traceback (most recent call last):
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/task/task_runner/standard_task_runner.py", line 85, in _start_by_fork
    args.func(args, dag=self.dag)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/cli_parser.py", line 48, in command
    return func(*args, **kwargs)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/utils/cli.py", line 92, in wrapper
    return f(*args, **kwargs)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/commands/task_command.py", line 298, in task_run
    _run_task_by_selected_method(args, dag, ti)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/commands/task_command.py", line 107, in _run_task_by_selected_method
    _run_raw_task(args, ti)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/cli/commands/task_command.py", line 180, in _run_raw_task
    ti._run_raw_task(
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/utils/session.py", line 70, in wrapper
    return func(*args, session=session, **kwargs)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1334, in _run_raw_task
    self._execute_task_with_callbacks(context)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1460, in _execute_task_with_callbacks
    result = self._execute_task(context, self.task)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/models/taskinstance.py", line 1516, in _execute_task
    result = execute_callable(context=context)
  File "/opt/airflow/dags/repo/plugins/ads_to_postgres.py", line 55, in execute
    rows = service.search(client_ids=self.client_ids,
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/google/ads/hooks/ads.py", line 113, in search
    """
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/google/ads/hooks/ads.py", line 234, in _search
  File "/home/airflow/.local/lib/python3.8/site-packages/google/ads/googleads/v10/services/services/google_ads_service/client.py", line 3306, in search
    request = google_ads_service.SearchGoogleAdsRequest(request)
  File "/home/airflow/.local/lib/python3.8/site-packages/proto/message.py", line 486, in __init__
    # Just use the above logic on mapping's underlying pb.
TypeError: Invalid constructor input for SearchGoogleAdsRequest: customer_id: "xxxxxxxxxxxx" 
query: "\n        SELECT customer.id,\n               customer.descriptive_name,\n               campaign.id, campaign.name, ad_group.id, ad_group.name,               \n               geographic_view.country_criterion_id,\n               segments.date, \n               metrics.cost_micros, metrics.impressions, metrics.clicks, metrics.conversions, metrics.ctr\n        FROM geographic_view\n        WHERE segments.date=\'2022-05-11\'\n    "
page_size: 10000

(I modify the real id)

[potiuk]

BTW. The google provider (including ads10 support) is being voted right now. I encourage everyone to test it and report status at #23659

[anthonyjgraff]

@jiamo

Have spent some time looking through your traceback & having a read of the code

You have this line in your traceback:

File "/home/airflow/.local/lib/python3.8/site-packages/google/ads/googleads/v10/services/services/google_ads_service/client.py", line 3306, in search

This line is only hit when:

• We have v10 of GoogleAdsServiceClient (returned by GoogleAdsHook._get_service)
• The request object isn't a v10 SearchGoogleAdsRequest (see line 3305 above)

As the search function creates a SearchGoogleAdsRequest object from the GoogleAdsClient returned by GoogleAdsHook._get_client, I'm guessing this means you've somehow got a v8 / v9 version of the client.

I haven't been able to create a scenario where the service is newer than the request object, but I did manage to create the reverse scenario where a v10 request object was sent to a v9 service and got the following, (which gave a pretty similar traceback)!

  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/google/ads/hooks/ads.py", line 114, in search
    data_proto_plus = self._search(client_ids, query, page_size, **kwargs)
  File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/google/ads/hooks/ads.py", line 235, in _search
    iterator = service.search(request=request)
  File "/home/airflow/.local/lib/python3.8/site-packages/google/ads/googleads/v9/services/services/google_ads_service/client.py", line 3163, in search
    request = google_ads_service.SearchGoogleAdsRequest(request)
  File "/home/airflow/.local/lib/python3.8/site-packages/proto/message.py", line 496, in __init__
    raise TypeError(
TypeError: Invalid constructor input for SearchGoogleAdsRequest: customer_id: "XXXXXXXXXXX"
query: "\n            SELECT\n                customer_client.id,\n                customer_client.status\n            FROM customer_client\n            WHERE customer_client.level <= 1\n            "
page_size: 10000

In my case, I suspect the bug was caused because we don't pass self.api_version through when calling GoogleAdsClient.load_from_dict in the GoogleAdsHook (meaning the client defaults to the latest version, whilst the service correctly uses v9).

From what I can tell, this behaviour long pre-dates this PR (so not sure how big of an issue it is with regards to #23659), but can investigate further tomorrow.

[jiamo]

@anthonyjgraff

It is strange I don't upgrade airflow and login in the worker pod. just execute pip install apache-airflow-providers-google==7.0.0rc. So when GoogleAdsClient.load_from_dict was called?

same logic code . just run python and paste

from operator import attrgetter
from tempfile import NamedTemporaryFile
from typing import Dict, List, Optional, Sequence

from airflow.models import BaseOperator
from airflow.plugins_manager import AirflowPlugin
from airflow.providers.google.ads.hooks.ads import GoogleAdsHook
query = """
        SELECT customer.id,
               customer.descriptive_name,
               campaign.id, campaign.name, ad_group.id, ad_group.name,               
               geographic_view.country_criterion_id,
               segments.date, 
               metrics.cost_micros, metrics.impressions, metrics.clicks, metrics.conversions, metrics.ctr
        FROM geographic_view
        WHERE segments.date=\'2022-05-11\'
    """
service = GoogleAdsHook(gcp_conn_id='onion-16040', google_ads_conn_id='google_ads_default',  api_version="v10")
rows = service.search(client_ids=["xxxxxxxxxxx"], query=query, page_size=10000)

It works fine. GoogleAdsHook and service.search was fine.
I suspect that only pip install don't have a total complete upgrade for v10?

[jiamo]

Re apply helm file with the latest 7.0.0rc works fine. So the problem only happened at just pip install 7.0.0rc.