diff --git a/ChangeLog b/ChangeLog index db9f27364..0d9b81b7f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +* 2.4.1: +- Fix bug preventing login_customer_id to be loaded as an int + * 2.4.0: - Add utf-8 encoding declaration in generated proto files - Add Service Account support diff --git a/google/ads/google_ads/__init__.py b/google/ads/google_ads/__init__.py index 92db5c1a3..7acdf6567 100644 --- a/google/ads/google_ads/__init__.py +++ b/google/ads/google_ads/__init__.py @@ -21,4 +21,4 @@ import google.ads.google_ads.util -VERSION = '2.4.0' +VERSION = '2.4.1' diff --git a/google/ads/google_ads/client.py b/google/ads/google_ads/client.py index 4c7788eb0..aaa0acb96 100644 --- a/google/ads/google_ads/client.py +++ b/google/ads/google_ads/client.py @@ -55,14 +55,10 @@ def _get_client_kwargs(cls, config_data): Raises: ValueError: If the configuration lacks a required field. """ - login_customer_id = config_data.get('login_customer_id') - login_customer_id = str( - login_customer_id) if login_customer_id else None - return {'credentials': oauth2.get_credentials(config_data), 'developer_token': config_data.get('developer_token'), 'endpoint': config_data.get('endpoint'), - 'login_customer_id': login_customer_id, + 'login_customer_id': config_data.get('login_customer_id'), 'logging_config': config_data.get('logging')} @classmethod diff --git a/google/ads/google_ads/config.py b/google/ads/google_ads/config.py index 3a03e090d..88652d059 100644 --- a/google/ads/google_ads/config.py +++ b/google/ads/google_ads/config.py @@ -47,6 +47,22 @@ def validation_wrapper(*args, **kwargs): return validation_wrapper +def _config_parser_decorator(func): + """A decorator used to easily parse config values. + + Since configs can be loaded from different locations such as env vars or + from YAML files it's possible that they may have inconsistent types that + need to be parsed to a different type. Add this decorator to any method + that returns the config as a dict. + """ + @functools.wraps(func) + def parser_wrapper(*args, **kwargs): + config_dict = func(*args, **kwargs) + parsed_config = convert_login_customer_id_to_str(config_dict) + return parsed_config + return parser_wrapper + + def validate_dict(config_data): """Validates the given configuration dict. @@ -87,6 +103,7 @@ def validate_login_customer_id(login_customer_id): @_config_validation_decorator +@_config_parser_decorator def load_from_yaml_file(path=None): """Loads configuration data from a YAML file and returns it as a dict. @@ -114,6 +131,7 @@ def load_from_yaml_file(path=None): @_config_validation_decorator +@_config_parser_decorator def parse_yaml_document_to_dict(yaml_doc): """Parses a YAML document to a dict. @@ -131,6 +149,7 @@ def parse_yaml_document_to_dict(yaml_doc): @_config_validation_decorator +@_config_parser_decorator def load_from_env(): """Loads configuration data from the environment and returns it as a dict. @@ -171,3 +190,24 @@ def get_oauth2_service_account_keys(): A tuple containing the required keys as strs. """ return _OAUTH2_SERVICE_ACCOUNT_KEYS + + +def convert_login_customer_id_to_str(config_data): + """Parses a config dict's login_customer_id attr value to a str. + + Like many values from YAML it's possible for login_customer_id to + either be a str or an int. Since we actually run validations on this + value before making requests it's important to parse it to a str. + + Args: + config_data: A config dict object. + + Returns: + The same config dict object with a mutated login_customer_id attr. + """ + login_customer_id = config_data.get('login_customer_id') + + if login_customer_id: + config_data['login_customer_id'] = str(login_customer_id) + + return config_data diff --git a/setup.py b/setup.py index 9a5377e61..d0512f34f 100644 --- a/setup.py +++ b/setup.py @@ -35,7 +35,7 @@ setup( name='google-ads', - version='2.4.0', + version='2.4.1', author='Google LLC', author_email='googleapis-packages@google.com', classifiers=[ diff --git a/tests/client_test.py b/tests/client_test.py index 5ecd019d4..81df3a2dc 100644 --- a/tests/client_test.py +++ b/tests/client_test.py @@ -232,6 +232,40 @@ def test_load_from_storage(self): login_customer_id=None, logging_config=None) + def test_load_from_storage_login_cid_int(self): + login_cid = 1234567890 + config = { + 'developer_token': self.developer_token, + 'client_id': self.client_id, + 'client_secret': self.client_secret, + 'refresh_token': self.refresh_token, + 'login_customer_id': login_cid} + + file_path = os.path.join(os.path.expanduser('~'), 'google-ads.yaml') + self.fs.create_file(file_path, contents=yaml.safe_dump(config)) + mock_credentials_instance = mock.Mock() + + with mock.patch.object( + Client.GoogleAdsClient, + '__init__', + return_value=None + ) as mock_client_init, mock.patch.object( + Client.oauth2, + 'get_installed_app_credentials', + return_value=mock_credentials_instance + ) as mock_credentials: + Client.GoogleAdsClient.load_from_storage() + mock_credentials.assert_called_once_with( + config.get('client_id'), + config.get('client_secret'), + config.get('refresh_token')) + mock_client_init.assert_called_once_with( + credentials=mock_credentials_instance, + developer_token=self.developer_token, + endpoint=None, + login_customer_id=str(login_cid), + logging_config=None) + def test_load_from_storage_custom_path(self): config = { 'developer_token': self.developer_token, diff --git a/tests/config_test.py b/tests/config_test.py index 050cd6d7e..c94392427 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -75,6 +75,23 @@ def test_load_from_yaml_file_with_path(self): self.assertEqual(result['client_secret'], self.client_secret) self.assertEqual(result['refresh_token'], self.refresh_token) + def test_load_from_yaml_file_login_cid_int(self): + login_cid_int = 1234567890 + file_path = os.path.join(os.path.expanduser('~'), 'google-ads.yaml') + self.fs.create_file(file_path, contents=yaml.safe_dump({ + 'login_customer_id': login_cid_int, + 'developer_token': self.developer_token, + 'client_id': self.client_id, + 'client_secret': self.client_secret, + 'refresh_token': self.refresh_token})) + + result = config.load_from_yaml_file() + + self.assertEqual(result['developer_token'], self.developer_token) + self.assertEqual(result['client_id'], self.client_id) + self.assertEqual(result['client_secret'], self.client_secret) + self.assertEqual(result['refresh_token'], self.refresh_token) + def test_parse_yaml_document_to_dict(self): yaml_doc = ('client_id: {}\n' 'client_secret: {}\n' @@ -199,3 +216,19 @@ def test_get_oauth2_installed_app_keys(self): def test_get_oauth2_service_account_keys(self): self.assertEqual(config.get_oauth2_service_account_keys(), config._OAUTH2_SERVICE_ACCOUNT_KEYS) + + def test_convert_login_customer_id_to_str_with_int(self): + config_data = {'login_customer_id': 1234567890} + expected = {'login_customer_id': '1234567890'} + self.assertEqual(config.convert_login_customer_id_to_str(config_data), + expected) + + def test_parse_login_customer_id_with_str(self): + config_data = {'login_customer_id': '1234567890'} + self.assertEqual(config.convert_login_customer_id_to_str(config_data), + config_data) + + def test_parse_login_customer_id_with_none(self): + config_data = {'not_login_customer_id': 1234567890} + self.assertEqual(config.convert_login_customer_id_to_str(config_data), + config_data)