Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade to support Google Ads v10 #22965

Merged
merged 5 commits into from
Apr 15, 2022
Merged

Upgrade to support Google Ads v10 #22965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
03d3c1b
Upgrade to support Google Ads v10
anthonyjgraff Apr 12, 2022
8ae2ffd
Update changelog
anthonyjgraff Apr 13, 2022
f159a92
Pin colorlog to <5
anthonyjgraff Apr 13, 2022
2092fca
Comment describing Colorlog pin
anthonyjgraff Apr 14, 2022
df6ddec
Delete requirements.txt
potiuk Apr 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1342,8 +1342,7 @@ ARG INSTALL_FROM_PYPI="true"
# Force them on the main Airflow package.
# * certifi<2021.0.0 required to keep snowflake happy
# * dill<0.3.3 required by apache-beam
# * google-ads<14.0.1 required to prevent updating google-python-api>=2.0.0
ARG EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS="dill<0.3.3 certifi<2021.0.0 google-ads<14.0.1"
ARG EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS="dill<0.3.3 certifi<2021.0.0"

ENV ADDITIONAL_PYTHON_DEPS=${ADDITIONAL_PYTHON_DEPS} \
INSTALL_FROM_DOCKER_CONTEXT_FILES=${INSTALL_FROM_DOCKER_CONTEXT_FILES} \
Expand Down
3 changes: 1 addition & 2 deletions Dockerfile.ci
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1588,8 +1588,7 @@ RUN echo "Airflow version: ${AIRFLOW_VERSION}"
# force them on the main Airflow package. Those limitations are:
# * certifi<2021.0.0: required by snowflake provider
# * dill<0.3.3 required by apache-beam
# * google-ads<14.0.1 required to prevent updating google-python-api>=2.0.0
ARG EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS="dill<0.3.3 certifi<2021.0.0 google-ads<14.0.1"
ARG EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS="dill<0.3.3 certifi<2021.0.0"
ARG UPGRADE_TO_NEWER_DEPENDENCIES="false"
ENV EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS=${EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS} \
UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
Expand Down
16 changes: 16 additions & 0 deletions airflow/providers/google/CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@
Changelog
---------

7.0.0
.....

Breaking changes
~~~~~~~~~~~~~~~~

* ``apache-airflow-providers-google uses deprecated Google Ads API V8 (#22111)``

.. warning:: The underlying google-ads library has been updated

This drops support for versions v6 and v7 of the Google Ads API, and updates
the default version of the Google Ads API from the deprecated v8 to v10.

For more information, see `Deprecation and sunset <https://developers.google.com/google-ads/api/docs/sunset-dates>`_
and `Upgrading to the newest version <https://developers.google.com/google-ads/api/docs/version-migration>`_

6.8.0
.....

Expand Down
23 changes: 12 additions & 11 deletions airflow/providers/google/ads/hooks/ads.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@

from google.ads.googleads.client import GoogleAdsClient
from google.ads.googleads.errors import GoogleAdsException
from google.ads.googleads.v8.services.types.google_ads_service import GoogleAdsRow
from google.ads.googleads.v10.services.services.customer_service import CustomerServiceClient
from google.ads.googleads.v10.services.services.google_ads_service import GoogleAdsServiceClient
from google.ads.googleads.v10.services.types.google_ads_service import GoogleAdsRow, SearchGoogleAdsRequest
from google.api_core.page_iterator import GRPCIterator
from google.auth.exceptions import GoogleAuthError
from googleapiclient.discovery import Resource

from airflow import AirflowException
from airflow.hooks.base import BaseHook
Expand All @@ -51,12 +52,12 @@ class GoogleAdsHook(BaseHook):
{
"google_ads_client": {
"developer_token": "{{ INSERT_TOKEN }}",
"path_to_private_key_file": null,
"delegated_account": "{{ INSERT_DELEGATED_ACCOUNT }}"
"json_key_file_path": null,
"impersonated_email": "{{ INSERT_IMPERSONATED_EMAIL }}"
}
}

The ``path_to_private_key_file`` is resolved by the hook using credentials from gcp_conn_id.
The ``json_key_file_path`` is resolved by the hook using credentials from gcp_conn_id.
https://developers.google.com/google-ads/api/docs/client-libs/python/oauth-service

.. seealso::
Expand All @@ -75,7 +76,7 @@ class GoogleAdsHook(BaseHook):
:rtype: list[GoogleAdsRow]
"""

default_api_version = "v8"
default_api_version = "v10"

def __init__(
self,
Expand Down Expand Up @@ -155,13 +156,13 @@ def list_accessible_customers(self) -> List[str]:
raise

@cached_property
def _get_service(self) -> Resource:
def _get_service(self) -> GoogleAdsServiceClient:
"""Connects and authenticates with the Google Ads API using a service account"""
client = self._get_client
return client.get_service("GoogleAdsService", version=self.api_version)

@cached_property
def _get_client(self) -> Resource:
def _get_client(self) -> GoogleAdsClient:
with NamedTemporaryFile("w", suffix=".json") as secrets_temp:
self._get_config()
self._update_config_with_secret(secrets_temp)
Expand All @@ -173,7 +174,7 @@ def _get_client(self) -> Resource:
raise

@cached_property
def _get_customer_service(self) -> Resource:
def _get_customer_service(self) -> CustomerServiceClient:
"""Connects and authenticates with the Google Ads API using a service account"""
with NamedTemporaryFile("w", suffix=".json") as secrets_temp:
self._get_config()
Expand Down Expand Up @@ -207,7 +208,7 @@ def _update_config_with_secret(self, secrets_temp: IO[str]) -> None:
secrets_temp.write(secret)
secrets_temp.flush()

self.google_ads_config["path_to_private_key_file"] = secrets_temp.name
self.google_ads_config["json_key_file_path"] = secrets_temp.name

def _search(
self, client_ids: List[str], query: str, page_size: int = 10000, **kwargs
Expand All @@ -226,7 +227,7 @@ def _search(

iterators = []
for client_id in client_ids:
request = self._get_client.get_type("SearchGoogleAdsRequest")
request = self._get_client.get_type("SearchGoogleAdsRequest") # type: SearchGoogleAdsRequest
request.customer_id = client_id
request.query = query
request.page_size = page_size
Expand Down
2 changes: 1 addition & 1 deletion docs/apache-airflow-providers-google/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ PIP package Version required
====================================== ====================
``apache-airflow`` ``>=2.1.0``
``PyOpenSSL``
``google-ads`` ``>=12.0.0,<14.0.1``
``google-ads`` ``>=15.1.1``
``google-api-core`` ``>=1.25.1,<3.0.0``
``google-api-python-client`` ``>=1.6.0,<2.0.0``
``google-auth-httplib2`` ``>=0.0.1``
Expand Down
4 changes: 3 additions & 1 deletion setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ install_requires =
# Cattrs upgrades were known to break lineage https://github.com/apache/airflow/issues/16172
# TODO: Cattrs is now at 3.8 version so we should attempt to upgrade cattrs soon.
cattrs~=1.1, !=1.7.*
colorlog>=4.0.2
# Colorlog 6.x merges TTYColoredFormatter into ColoredFormatter, breaking backwards compatibility with 4.x
# Update CustomTTYColoredFormatter to remove
colorlog>=4.0.2, <5.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@potiuk potiuk Apr 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

connexion[swagger-ui,flask]>=2.10.0
cron-descriptor>=1.2.24
croniter>=0.3.17
Expand Down
5 changes: 1 addition & 4 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,7 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
# Introduced breaking changes across the board. Those libraries should be upgraded soon
# TODO: Upgrade all Google libraries that are limited to <2.0.0
'PyOpenSSL',
# The Google Ads 14.0.1 breaks PIP and eager upgrade as it requires
# google-api-core>=2.0.0 which cannot be used yet (see below comment)
# and https://github.com/apache/airflow/issues/18705#issuecomment-933746150
'google-ads>=12.0.0,<14.0.1',
'google-ads>=15.1.1',
'google-api-core>=2.7.0,<3.0.0',
'google-api-python-client>=1.6.0,<2.0.0',
'google-auth>=1.0.0',
Expand Down
2 changes: 1 addition & 1 deletion tests/providers/google/ads/operators/test_ads.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

gcp_conn_id = "gcp_conn_id"
google_ads_conn_id = "google_ads_conn_id"
api_version = "v8"
api_version = "v10"


class TestGoogleAdsListAccountsOperator:
Expand Down