-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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.
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
🎉 Source Amazon Ads: filters for state on brand, product and display campaigns #17475
🎉 Source Amazon Ads: filters for state on brand, product and display campaigns #17475
Conversation
7eb5d0c
to
b1e480d
Compare
@sage-watterworth can you solve the conflict? |
87cf7f6
to
256901f
Compare
marcosmarxm Merge conflict resolved |
@sage-watterworth sorry for the long wait time with this PR. We're hoping to get this merged within the next day or two! |
@sajarin Great news! Thanks for all your work on this. |
Hi @sajarin ! Any updates on when we can merge this PR? Thank you! |
Hi @sajarin just wanted to follow-up here and see when we could expect the PR to be merged? Thanks! |
/test connector=connectors/source-amazon-ads
Build FailedTest summary info:
|
in Report Streamsconfig: secrets/config_report.json Non Report Streamconfig: secrets/config.json If we compare
We split this, because for report streams multiple profiles is too slow, I see this PR add some additional filter option |
You also need to merge master branch to your branch to simplify review process |
@grubberr You'll find in this PRs comments why we originally reconfigured the acceptance test file. |
@grubberr just merged master into this branch |
|
|
||
|
||
@pytest.mark.parametrize("state_filter", [["enabled", "archived", "paused"], ["enabled"], None]) | ||
def test_display_report_stream_product_campaign_state_filer(mocker, config, state_filter): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test idential to previous test_display_report_stream_campaign_state_filer
can we merge all this 3 tests to one and parameterize class name: SponsoredDisplayCampaigns, SponsoredProductCampaigns, SponsoredBrandsCampaigns
/publish connector=connectors/source-amazon-ads
if you have connectors that successfully published but failed definition generation, follow step 4 here |
/publish connector=connectors/source-amazon-ads
if you have connectors that successfully published but failed definition generation, follow step 4 here |
/publish connector=connectors/source-amazon-ads
if you have connectors that successfully published but failed definition generation, follow step 4 here |
/publish connector=connectors/source-amazon-ads
if you have connectors that successfully published but failed definition generation, follow step 4 here |
Thanks for the PR @sage-watterworth @coltonpomeroy! We immensely appreciate all of your patience. :) |
…campaigns (airbytehq#17475) * amazon ad status filter * update test report stream with parametrized campaigns * remove config files related to previous acceptance test. revert to master acceptance test set up * amazon ad status filter * update test report stream with parametrized campaigns * remove config files related to previous acceptance test. revert to master acceptance test set up * oct 17 edits * oct 17 edits: 2 * bump dockerfile to 1.24 * fix: match the cdk version to new one * auto-bump connector version Co-authored-by: sajarin <sajarindider@gmail.com> Co-authored-by: Harshith Mullapudi <harshithmullapudi@interviewkickstart.com> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
What
Adding an option for a filter (by state) in the Amazon Ads connection:
This PR will allow Brand, Product and Display campaigns to use the status (state) filter.
How
Adding the field to the spec.yaml, and accessing it by adding to the path to Amazon Ads call as a query parameter.
Adding request params to Brand, Product and Display campaigns.
Recommended reading order
sponsored_brands.py
sponsored_product.py
sponsored_display.py
test_report_streams.py
🚨 User Impact 🚨
The parameter is optional and will not affect current functionality.
Pre-merge Checklist
Expand the relevant checklist and delete the others.
Updating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereTests
Unit
source_amazon_ads/utils.py 9 0 100%
source_amazon_ads/streams/sponsored_products.py 41 0 100%
source_amazon_ads/streams/sponsored_display.py 31 0 100%
source_amazon_ads/streams/sponsored_brands.py 26 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/init.py 6 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/init.py 6 0 100%
source_amazon_ads/constants.py 1 0 100%
source_amazon_ads/init.py 2 0 100%
source_amazon_ads/streams/common.py 76 1 99%
source_amazon_ads/schemas/common.py 51 1 98%
source_amazon_ads/streams/report_streams/report_streams.py 240 18 92%
source_amazon_ads/source.py 38 8 79%
TOTAL 713 28 96%
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.