Merge branch 'master' into TDL-24718/python-upgrade

.circleci/config.yml

@@ -18,8 +18,7 @@ jobs:
           name: 'pylint'
           command: |
             source /usr/local/share/virtualenvs/tap-bing-ads/bin/activate
-            pylint tap_bing_ads -d missing-docstring,line-too-long,invalid-name,super-with-arguments,return-in-init,too-many-arguments,deprecated-method,consider-using-f-string,too-many-lines,unidiomatic-typecheck
-      - add_ssh_keys
+            pylint tap_bing_ads -d missing-docstring,line-too-long,invalid-name,super-with-arguments,return-in-init,too-many-arguments,deprecated-method,consider-using-f-string,too-many-lines,unidiomatic-typecheck,consider-using-generator
       - run:
           name: 'Unit Tests'
           command: |
@@ -35,10 +34,14 @@ jobs:
           command: |
             aws s3 cp s3://com-stitchdata-dev-deployment-assets/environments/tap-tester/tap_tester_sandbox dev_env.sh
             source dev_env.sh
+            mkdir /tmp/${CIRCLE_PROJECT_REPONAME}
+            export STITCH_CONFIG_DIR=/tmp/${CIRCLE_PROJECT_REPONAME}
             source /usr/local/share/virtualenvs/tap-tester/bin/activate
             run-test --tap=tap-bing-ads tests
       - slack/notify-on-failure:
           only_for_branches: master
+      - store_artifacts:
+          path: /tmp/tap-bing-ads
 workflows:
   version: 2
   commit: &commit_jobs

CHANGELOG.md

@@ -1,13 +1,15 @@
 # Changelog
 
-## 2.20
+## 2.2.1
+  * Prefer new authentication scope for the access_token request [#99](https://github.com/singer-io/tap-bing-ads/pull/99)
+
+## 2.2.0
   * Implement Timeout Request [#93](https://github.com/singer-io/tap-bing-ads/pull/93)
   * Added Top Level Breadcrumb [#91](https://github.com/singer-io/tap-bing-ads/pull/91)
   * Added Retry Logic for 500 errors [#90](https://github.com/singer-io/tap-bing-ads/pull/90)
   * Added required fields for age_gender_audience_report [#86](https://github.com/singer-io/tap-bing-ads/pull/86)
   * Removed automatic fields from ad_ext report [#85](https://github.com/singer-io/tap-bing-ads/pull/85)
 
-
 ## 2.1.0
   * Update the BingAds library to `13.0.11`
 

setup.py

@@ -12,15 +12,17 @@
     py_modules=['tap_bingads'],
     install_requires=[
         'arrow==0.12.0',
-        'bingads==13.0.11',
+        # Seems that suds-community is now the reference for 13.0.11.1 so we can install it now with the removal of use_2to3
+        # https://github.com/BingAds/BingAds-Python-SDK/pull/192
+        'bingads==13.0.11.1',
         'requests==2.31.0',
         'singer-python==6.0.0',
         'stringcase==1.2.0',
         'backoff==2.2.1',
     ],
     extras_require={
         'test': [
-            'pylint'
+            'pylint==3.0.3'
         ],
         'dev': [
             'ipdb'

tap_bing_ads/__init__.py

@@ -15,6 +15,7 @@
 from urllib.error import URLError
 import singer
 from singer import utils, metadata, metrics
+import bingads
 from bingads import AuthorizationData, OAuthWebAuthCodeGrant, ServiceClient
 import suds
 from suds.sudsobject import asdict
@@ -152,26 +153,38 @@ def set_options(self, **kwargs):
         kwargs['timeout'] = get_request_timeout()
         self._soap_client.set_options(**kwargs)
 
+def get_authentication():
+    """
+    Authenticate using the new method (no scope specified) and
+    fall back to the legacy method using the bingads.manage scope if
+    that fails.
+    """
+    # Represents an OAuth authorization object implementing the authorization code grant flow for use in a web application.
+    try:
+        authentication = OAuthWebAuthCodeGrant(
+            CONFIG['oauth_client_id'],
+            CONFIG['oauth_client_secret'],
+            '') ## redirect URL not needed for refresh token
+        # Retrieves OAuth access and refresh tokens from the Microsoft Account authorization service.
+        authentication.request_oauth_tokens_by_refresh_token(CONFIG['refresh_token'])
+        return authentication
+    except bingads.exceptions.OAuthTokenRequestException:
+        authentication = OAuthWebAuthCodeGrant(
+            CONFIG['oauth_client_id'],
+            CONFIG['oauth_client_secret'],
+            '',
+            oauth_scope='bingads.manage') ## redirect URL not needed for refresh token
+        # Retrieves OAuth access and refresh tokens from the Microsoft Account authorization service.
+        authentication.request_oauth_tokens_by_refresh_token(CONFIG['refresh_token'])
+        return authentication
+
 @bing_ads_error_handling
 def create_sdk_client(service, account_id):
     # Creates SOAP client with OAuth refresh credentials for services
     LOGGER.info('Creating SOAP client with OAuth refresh credentials for service: %s, account_id %s',
                 service, account_id)
 
-    if CONFIG.get('require_live_connect', 'True') == 'True':
-        oauth_scope = 'msads.manage'#'bingads.manage'
-    else:
-        oauth_scope = 'msads.manage'
-
-    # Represents an OAuth authorization object implementing the authorization code grant flow for use in a web application.
-    authentication = OAuthWebAuthCodeGrant(
-        CONFIG['oauth_client_id'],
-        CONFIG['oauth_client_secret'],
-        '',
-        oauth_scope=oauth_scope) ## redirect URL not needed for refresh token
-
-    # Retrieves OAuth access and refresh tokens from the Microsoft Account authorization service.
-    authentication.request_oauth_tokens_by_refresh_token(CONFIG['refresh_token'])
+    authentication = get_authentication()
 
     # Instance require to authenticate with Bing Ads
     authorization_data = AuthorizationData(

tests/base.py

@@ -2,15 +2,15 @@
 Setup expectations for test sub classes
 Run discovery for as a prerequisite for most tests
 """
-import unittest
 import backoff
 import copy
 import os
 from datetime import timedelta
 from datetime import datetime as dt
 from datetime import timezone as tz
 
-from tap_tester import connections, menagerie, runner
+from tap_tester import connections, menagerie, runner, LOGGER
+from tap_tester.base_case import BaseCase
 
 def backoff_wait_times():
     """Create a generator of wait times as [30, 60, 120, 240, 480, ...]"""
@@ -22,7 +22,7 @@ def __init__(self, message):
         super().__init__(message)
 
 
-class BingAdsBaseTest(unittest.TestCase):
+class BingAdsBaseTest(BaseCase):
     """
     Setup expectations for test sub classes
     Run discovery for as a prerequisite for most tests
@@ -222,7 +222,7 @@ def run_check_mode(self, conn_id):
             menagerie.verify_check_exit_status(self, exit_status, check_job_name)
         except AssertionError as e:
             if exit_status['discovery_error_message']:
-                print("*******************RETRYING CHECK FOR DISCOVERY FAILURE*******************")
+                LOGGER.warn("*******************RETRYING CHECK FOR DISCOVERY FAILURE*******************")
                 raise RetryableTapError(e)
 
             raise
@@ -233,7 +233,7 @@ def verify_check_mode(self, conn_id):
 
         found_catalog_names = set(map(lambda c: c['tap_stream_id'], found_catalogs))
         self.assertSetEqual(self.expected_streams(), found_catalog_names, msg="discovered schemas do not match")
-        print("discovered schemas are OK")
+        LOGGER.info("discovered schemas are OK")
         return found_catalogs
 
     def run_and_verify_check_mode(self, conn_id):
@@ -267,7 +267,7 @@ def run_and_verify_sync(self, conn_id, state):
             menagerie.verify_sync_exit_status(self, exit_status, sync_job_name)
         except AssertionError as e:
             if exit_status['discovery_error_message'] or exit_status['tap_error_message']:
-                print("*******************RETRYING SYNC FOR TAP/DISCOVERY FAILURE*******************")
+                LOGGER.warn("*******************RETRYING SYNC FOR TAP/DISCOVERY FAILURE*******************")
                 raise RetryableTapError(e)
 
             raise
@@ -279,7 +279,7 @@ def run_and_verify_sync(self, conn_id, state):
             sum(sync_record_count.values()), 0,
             msg="failed to replicate any data: {}".format(sync_record_count)
         )
-        print("total replicated row count: {}".format(sum(sync_record_count.values())))
+        LOGGER.info("total replicated row count: %s", sum(sync_record_count.values()))
 
         return sync_record_count
 
@@ -372,7 +372,7 @@ def perform_and_verify_table_and_field_selection(self,
 
             # Verify all testable streams are selected
             selected = catalog_entry.get('annotated-schema').get('selected')
-            print("Validating selection on {}: {}".format(cat['stream_name'], selected))
+            LOGGER.info("Validating selection on %s: %s", cat['stream_name'], selected)
             if cat['stream_name'] not in expected_selected:
                 self.assertFalse(selected, msg="Stream selected, but not testable.")
                 continue # Skip remaining assertions if we aren't selecting this stream
@@ -382,8 +382,7 @@ def perform_and_verify_table_and_field_selection(self,
                 # Verify all fields within each selected stream are selected
                 for field, field_props in catalog_entry.get('annotated-schema').get('properties').items():
                     field_selected = field_props.get('selected')
-                    print("\tValidating selection on {}.{}: {}".format(
-                        cat['stream_name'], field, field_selected))
+                    LOGGER.info("\tValidating selection on %s.%s: %s", cat['stream_name'], field, field_selected)
                     self.assertTrue(field_selected, msg="Field not selected.")
             else:
                 # Verify only automatic fields are selected
@@ -397,7 +396,7 @@ def get_selected_fields_from_metadata(metadata):
         for field in metadata:
             is_field_metadata = len(field['breadcrumb']) > 1
             if field['metadata'].get('inclusion') is None and is_field_metadata:  # BUG_SRCE-4313 remove when addressed
-                print("Error {} has no inclusion key in metadata".format(field))  # BUG_SRCE-4313 remove when addressed
+                LOGGER.info("Error %s has no inclusion key in metadata", field)  # BUG_SRCE-4313 remove when addressed
                 continue  # BUG_SRCE-4313 remove when addressed
             inclusion_automatic_or_selected = (
                 field['metadata']['selected'] is True or \
@@ -503,7 +502,7 @@ def perform_and_verify_adjusted_selection(self,
 
                 # Verify intended streams are selected
                 selected = catalog_entry.get('annotated-schema').get('selected')
-                print("Validating selection on {}: {}".format(cat['tap_stream_id'], selected))
+                LOGGER.info("Validating selection on %s: %s", cat['tap_stream_id'], selected)
                 if cat['stream_name'] not in expected_selected:
                     continue  # Skip remaining assertions if we aren't selecting this stream
 
@@ -513,15 +512,13 @@ def perform_and_verify_adjusted_selection(self,
                     # Verify all fields within each selected stream are selected
                     for field, field_props in catalog_entry.get('annotated-schema').get('properties').items():
                         field_selected = field_props.get('selected')
-                        print("\tValidating selection on {}.{}: {}".format(
-                            cat['stream_name'], field, field_selected))
+                        LOGGER.info("\tValidating selection on %s.%s: %s", cat['stream_name'], field, field_selected)
                         self.assertTrue(field_selected, msg="Field not selected.")
                 else:
                     for field, field_props in catalog_entry.get('annotated-schema').get('properties').items():
                         field_selected = field_props.get('selected')
                         if field_selected:
-                            print("\tValidating selection on {}.{}: {}".format(
-                                cat['stream_name'], field, field_selected))
+                            LOGGER.info("\tValidating selection on %s.%s: %s", cat['stream_name'], field, field_selected)
 
                     # Verify only automatic fields are selected
                     # Uncomment lines below to reporduce BUG_SRCE-4313 from automatic fields tests