1568 Google Ads best practices #1726
Conversation
…ementing incremental sync
…ld in spec.json
|
/test connector=source-google-adwords-singer
|
| from base_python import AirbyteLogger | ||
| from base_singer import SingerSource, SyncMode, SyncModeInfo | ||
| from googleads import adwords, oauth2 | ||
| from tap_adwords import VERSION | ||
|
|
||
|
|
||
| class SourceGoogleAdwordsSinger(SingerSource): |
There was a problem hiding this comment.
You can use BaseSingerSourse, this will slightly reduce the amount of code, for example, it will remove the disсover_сmd and the check_config functions, WDYT?
| accounts = self._get_accounts(logger, sdk_client, selector) | ||
| if not accounts: | ||
| return AirbyteConnectionStatus(status=Status.FAILED) | ||
| return AirbyteConnectionStatus(status=Status.SUCCEEDED) | ||
| except Exception as e: | ||
| return AirbyteConnectionStatus(status=Status.FAILED, message=f"{str(e)}") |
There was a problem hiding this comment.
The use of BaseSingerSource will display a universal and understandable wording in UI in case of an error.
| } | ||
| accounts = self._get_accounts(logger, sdk_client, selector) | ||
| if not accounts: | ||
| return AirbyteConnectionStatus(status=Status.FAILED) |
There was a problem hiding this comment.
We should always have a message set if the connection check fails. What should the error message here say?
| logger.log_by_prefix(f"Unable to sync {stream} with the manager account {customer_id}", "ERROR") | ||
| sys.exit(1) | ||
| else: | ||
| logger.log_by_prefix(f"Unable to sync with the provided customer id {customer_id}", "ERROR") |
There was a problem hiding this comment.
Can this log also specify a reason why it's failing? What should the user who sees this message do?
…68_google_ads_best_practices
…68_google_ads_best_practices
…68_google_ads_best_practices
|
created #1805 with follow up actions for this PR |
…68_google_ads_best_practices
What
Apply connector best practices for Salesforce source
How
Pre-merge Checklist