Source Amazon Ads: added `adId` to product report stream #11660

agroh1 · 2022-04-01T13:15:08Z

What

#11659 sponsored_products_report_stream does not return adId despite it being added automatically by amazon

How

Added adId to the request fields so that it will show up in the report sync but don't send it as a requested field to amazon

🚨 User Impact 🚨

adId field now returned for appropriate reports in sponsored_products_report_stream

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the new connector version is published, connector version bumped in the seed directory as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Results (31.92s):
39 passed

Integration

{"type": "RECORD", "record": {"stream": "sponsored_products_report_stream", "data": {"profileId": 3151297663604935, "recordType": "productAds", "reportDate": "20220401", "metric": {"campaignRuleBasedBudget": null, "attributedConversions1dSameSKU": "0", "attributedSales1d": "0", "bidPlus": null, "attributedConversions30dSameSKU": "0", "attributedSales14d": "0", "targetingExpression": null, "attributedSales7d": "0", "campaignBudget": null, "asin": "B07VF2K5DS", "impressions": "0", "campaignStatus": null, "applicableBudgetRuleName": null, "attributedSales14dOtherSKU": null, "attributedSales1dOtherSKU": null, "attributedConversions30d": "0", "attributedSales30dSameSKU": "0", "cost": "0", "attributedConversions14d": "0", "attributedSales7dSameSKU": "0", "applicableBudgetRuleId": null, "attributedUnitsOrdered1dSameSKU": "0", "targetId": null, "attributedUnitsOrdered30d": "0", "currency": "USD", "attributedSales1dSameSKU": "0", "attributedUnitsOrdered14d": "0", "attributedSales14dSameSKU": "0", "attributedConversions7d": "0", "keywordId": null, "attributedUnitsOrdered14dOtherSKU": null, "attributedSales7dOtherSKU": null, "adGroupId": "205722450325390", "attributedUnitsOrdered7dOtherSKU": null, "attributedUnitsOrdered30dSameSKU": "0", "adId": "220368155192190", "keywordText": null, "attributedSales30d": "0", "attributedConversions1d": "0", "campaignId": "128525041765967", "attributedUnitsOrdered14dSameSKU": "0", "attributedUnitsOrdered1d": "0", "attributedConversions14dSameSKU": "0", "attributedUnitsOrdered30dOtherSKU": null, "sku": null, "attributedConversions7dSameSKU": "0", "adGroupName": "Broad", "attributedSales30dOtherSKU": null, "clicks": "0", "attributedUnitsOrdered7dSameSKU": "0", "matchType": null, "targetingText": null, "otherAsin": null, "targetingType": null, "attributedUnitsOrdered7d": "0", "attributedUnitsOrdered1dOtherSKU": null, "campaignName": "Beard Oil | Manual | Unscented"}}, "emitted_at": 1648817346497}} {"type": "asinSameAs", "value": "B07RP1P6HQ"}]}, "emitted_at": 1648818018680}} {"type": "LOG", "log": {"level": "INFO", "message": "Read 423 records from sponsored_product_targetings stream"}} {"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing sponsored_product_targetings"}} {"type": "LOG", "log": {"level": "INFO", "message": "SourceAmazonAds runtimes:\nSyncing stream profiles 0:00:00.001417\nSyncing stream sponsored_display_ad_groups 0:00:00.270174\nSyncing stream sponsored_display_campaigns 0:00:00.247865\nSyncing stream sponsored_display_targetings 0:00:01.517041\nSyncing stream sponsored_product_ad_groups 0:00:00.397415\nSyncing stream sponsored_product_campaigns 0:00:00.363708\nSyncing stream sponsored_product_keywords 0:00:20.981859\nSyncing stream sponsored_product_negative_keywords 0:00:16.667616\nSyncing stream sponsored_product_targetings 0:00:00.978489\nSyncing stream sponsored_products_report_stream 0:05:08.647590"}} {"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceAmazonAds"}}

Acceptance

Put your acceptance tests output here.

CLAassistant · 2022-04-01T13:15:31Z

All committers have signed the CLA.

marcosmarxm · 2022-04-04T22:19:25Z

@agroh1 can you sign the CLA?

agroh1 · 2022-04-04T22:41:34Z

I did sign the CLA for agroh1. I accidentially had the user set incorrectly for the commit and I am trying to figure out how change the user on the commit

agroh1 · 2022-04-05T12:53:31Z

I have rebased the commit so it is now as agroh1 and agroh1 has signed the CLA. agroh is still listed as a contributor but is not on either the repo or any commit

marcosmarxm

thanks @agroh1 I think we can refactor to become simpler the change, wdyt?

marcosmarxm · 2022-04-05T17:23:22Z

...ons/connectors/source-amazon-ads/source_amazon_ads/streams/report_streams/products_report.py

+
+# adId is automatically added to the report by amazon and requesting adId causes an amazon error
        if RecordType.ASINS in record_type:
            body["campaignType"] = "sponsoredProducts"
+            metrics_list = copy(metrics_list)
+            metrics_list.remove("adId") 
            if profile.accountInfo.type == "vendor":
                metrics_list = copy(metrics_list)
                metrics_list.remove("sku")
+        elif RecordType.PRODUCTADS in record_type:
+            metrics_list = copy(metrics_list)
+            metrics_list.remove("adId")
+


# adId is automatically.... if "adId" in metrics_list: metrics_list.remove("adId")

This way if you add a new report you don't need to change the code again to make it works.
Why is the reason you're using copy?

I think that is probably better. It does assume that all product reports have this problem but I think that is probably correct.

I made a copy because the old code that removed "sku" also made a copy and I assumed that it was probably the right thing.

For those cases we can improve the if logic

if "adId" in metrics_list and not SpecialReport: metrics_list.remove("adId")

because right now both need to remove it right?

agroh1 · 2022-04-08T15:53:09Z

I updated and retested based on your suggestions

marcosmarxm · 2022-04-08T20:22:52Z

@agroh1 can you update the Dockerfile version?

agroh1 · 2022-04-08T20:26:58Z

Bumped dockerfile

marcosmarxm · 2022-04-19T00:40:22Z

/test connector=connectors/source-amazon-ads

🕑 connectors/source-amazon-ads https://github.com/airbytehq/airbyte/actions/runs/2186855122
✅ connectors/source-amazon-ads https://github.com/airbytehq/airbyte/actions/runs/2186855122
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              275    106    61%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  876    259    70%
Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
source_amazon_ads/streams/sponsored_products.py                      32      0   100%
source_amazon_ads/streams/sponsored_display.py                       22      0   100%
source_amazon_ads/streams/sponsored_brands.py                        17      0   100%
source_amazon_ads/streams/report_streams/products_report.py          18      0   100%
source_amazon_ads/streams/report_streams/display_report.py           16      0   100%
source_amazon_ads/streams/report_streams/brands_video_report.py      10      0   100%
source_amazon_ads/streams/report_streams/brands_report.py            10      0   100%
source_amazon_ads/streams/report_streams/__init__.py                  5      0   100%
source_amazon_ads/streams/profiles.py                                21      0   100%
source_amazon_ads/streams/common.py                                  78      0   100%
source_amazon_ads/streams/__init__.py                                 6      0   100%
source_amazon_ads/spec.py                                            25      0   100%
source_amazon_ads/schemas/sponsored_products.py                      37      0   100%
source_amazon_ads/schemas/sponsored_display.py                       31      0   100%
source_amazon_ads/schemas/sponsored_brands.py                        22      0   100%
source_amazon_ads/schemas/profile.py                                 16      0   100%
source_amazon_ads/schemas/common.py                                  45      0   100%
source_amazon_ads/schemas/__init__.py                                 6      0   100%
source_amazon_ads/constants.py                                        7      0   100%
source_amazon_ads/__init__.py                                         2      0   100%
source_amazon_ads/source.py                                          34      1    97%
source_amazon_ads/streams/report_streams/report_streams.py          199      6    97%
-------------------------------------------------------------------------------------
TOTAL                                                               659      7    99%

Python short test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.7/site-packages/source_acceptance_test/plugin.py:56: Skipping TestIncremental.test_two_sequential_reads because not found in the config
======================== 22 passed, 1 skipped in 16.13s ========================

…est-pr-11659

marcosmarxm · 2022-04-21T00:04:33Z

/publish connector=connectors/source-amazon-ads auto-bump-version=false

🕑 connectors/source-amazon-ads https://github.com/airbytehq/airbyte/actions/runs/2198979520
❌ Failed to publish connectors/source-amazon-ads

marcosmarxm · 2022-04-26T00:59:34Z

/publish connector=connectors/source-amazon-ads auto-bump-version=false

🕑 connectors/source-amazon-ads https://github.com/airbytehq/airbyte/actions/runs/2223777967
🚀 Successfully published connectors/source-amazon-ads
✅ connectors/source-amazon-ads https://github.com/airbytehq/airbyte/actions/runs/2223777967

marcosmarxm

thanks @agroh1

* added adId to product report stream * simplified removal of adId * removed duplicate comment * Bumped dockerfile version * bump connector version Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>

added adId to product report stream

01c3c88

github-actions bot added the area/connectors Connector related issues label Apr 1, 2022

octavia-squidington-iii added the community label Apr 1, 2022

Merge branch 'airbytehq:master' into issue-11659

376c56f

marcosmarxm requested changes Apr 5, 2022

View reviewed changes

marcosmarxm self-assigned this Apr 5, 2022

agroh1 added 2 commits April 8, 2022 10:58

simplified removal of adId

00ccf78

removed duplicate comment

44ae708

Bumped dockerfile version

b9c7dc1

marcosmarxm changed the title ~~added adId to product report stream~~ Source Amazon Ads: added adId to product report stream Apr 19, 2022

marcosmarxm added 2 commits April 20, 2022 19:21

Merge branch 'issue-11659' of github.com:agroh1/airbyte into marcos/t…

46ddcf7

…est-pr-11659

bump connector version

5f87976

agroh1 requested a review from a team as a code owner April 20, 2022 22:24

marcosmarxm removed the request for review from a team April 20, 2022 22:25

github-actions bot added area/api Related to the api area/documentation Improvements or additions to documentation area/frontend area/platform issues related to the platform area/scheduler area/server area/worker Related to worker CDK Connector Development Kit labels Apr 20, 2022

github-actions bot added kubernetes normalization labels Apr 20, 2022

Merge branch 'master' into marcos/test-pr-11659

0dfa74a

github-actions bot removed area/scheduler CDK Connector Development Kit area/api Related to the api kubernetes area/server area/worker Related to worker area/frontend normalization area/platform issues related to the platform labels Apr 21, 2022

marcosmarxm approved these changes Apr 26, 2022

View reviewed changes

marcosmarxm merged commit 3414656 into airbytehq:master Apr 26, 2022

octavia-squidington-iii mentioned this pull request Apr 27, 2022

Bump Airbyte version from 0.36.3-alpha to 0.36.4-alpha #12415

Merged

grubberr mentioned this pull request May 8, 2022

sponsored_products_report_stream does not return adId despite it being added automatically by amazon #11659

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source Amazon Ads: added `adId` to product report stream #11660

Source Amazon Ads: added `adId` to product report stream #11660

agroh1 commented Apr 1, 2022

CLAassistant commented Apr 1, 2022 •

edited

Loading

marcosmarxm commented Apr 4, 2022

agroh1 commented Apr 4, 2022

agroh1 commented Apr 5, 2022

marcosmarxm left a comment

marcosmarxm Apr 5, 2022

agroh1 Apr 5, 2022

marcosmarxm Apr 5, 2022

agroh1 commented Apr 8, 2022

marcosmarxm commented Apr 8, 2022

agroh1 commented Apr 8, 2022

marcosmarxm commented Apr 19, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Apr 21, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Apr 26, 2022 •

edited by github-actions bot

Loading

marcosmarxm left a comment

Source Amazon Ads: added adId to product report stream #11660

Source Amazon Ads: added adId to product report stream #11660

Conversation

agroh1 commented Apr 1, 2022

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

CLAassistant commented Apr 1, 2022 • edited Loading

marcosmarxm commented Apr 4, 2022

agroh1 commented Apr 4, 2022

agroh1 commented Apr 5, 2022

marcosmarxm left a comment

Choose a reason for hiding this comment

marcosmarxm Apr 5, 2022

Choose a reason for hiding this comment

agroh1 Apr 5, 2022

Choose a reason for hiding this comment

marcosmarxm Apr 5, 2022

Choose a reason for hiding this comment

agroh1 commented Apr 8, 2022

marcosmarxm commented Apr 8, 2022

agroh1 commented Apr 8, 2022

marcosmarxm commented Apr 19, 2022 • edited by github-actions bot Loading

marcosmarxm commented Apr 21, 2022 • edited by github-actions bot Loading

marcosmarxm commented Apr 26, 2022 • edited by github-actions bot Loading

marcosmarxm left a comment

Choose a reason for hiding this comment

Source Amazon Ads: added `adId` to product report stream #11660

Source Amazon Ads: added `adId` to product report stream #11660

CLAassistant commented Apr 1, 2022 •

edited

Loading

marcosmarxm commented Apr 19, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Apr 21, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Apr 26, 2022 •

edited by github-actions bot

Loading