-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 and product campaigns #16829
🎉 Source Amazon Ads: filters for state on brand and product campaigns #16829
Conversation
… order to use state filter
Hey @sage-watterworth, thanks for making another PR related to this connector, we really appreciate the awesome work you've been doing! This PR is part of Airbyte's Community Maintainer program and will be reviewed by someone from our community shortly. We hope to get this merged into main soon, thanks for being patient! |
Hey @sage-watterworth, thanks for the wait. I'm escalating this PR to our dev team. Thanks for being patient! |
sajarin thank you for the update! |
@@ -554,3 +557,42 @@ def test_read_incremental_with_records_start_date(config): | |||
records = list(read_incremental(stream, state)) | |||
assert state == {"1": {"reportDate": "20210104"}} | |||
assert {r["reportDate"] for r in records} == {"20210103", "20210104", "20210105", "20210106"} | |||
|
|||
|
|||
@pytest.mark.parametrize("state_filter", [["enabled", "archived", "paused"], ["enabled"], None]) |
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.
Now, these three tests are the same except for the class name. Use itertools.product
to generate test cases for all state filter and stream class pairs.
from itertools import product
@pytest.mark.parametrize("state_filter", "klass", product(
[["enabled", "archived", "paused"], ["enabled"], None],
[SponsoredDisplayCampaigns, SponsoredProductCampaigns, SponsoredBrandsCampaigns]
))
def test_campaign_state_filer(mocker, config, state_filter, klass):
...
mocker.patch.object(klass, "state_filter", new_callable=mocker.PropertyMock, return_value=state_filter)
...
model = DisplayCampaign | ||
|
||
def path(self, **kvargs) -> str: | ||
def path(self, **kwargs) -> str: |
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.
Fix kwargs
naming in all the modified files.
@@ -12,12 +12,23 @@ class SponsoredBrandsCampaigns(SubProfilesStream): | |||
https://advertising.amazon.com/API/docs/en-us/sponsored-brands/3-0/openapi#/Campaigns | |||
""" | |||
|
|||
def __init__(self, *args, **kwargs): |
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.
The use of state_filter
parameter is copy-pasted across all three streams. Refactor it to a mixin instead, e.g.
class StateFilterMixin:
state_filter = None
def __init__(self, *args, **kwargs):
self.state_filter = kwargs.get("config", {}).get("state_filter")
super().__init__(*args, **kwargs)
def request_params(self, *args, **kwargs):
params = super().request_params(*args, **kwargs)
if self.state_filter:
params["stateFilter"] = ",".join(self.state_filter)
return params
class SponsoredBrandsCampaigns(StateFilterMixin, SubProfilesStream):
...
Place StateFilterMixin
in common.py
.
@sage-watterworth @monai |
Oh, it looks like it duplicates #17475, indeed. I'm closing it, then. |
What
This PR will allow Brand and Product campaigns to use the status (state) filter. This is an iteration of the following PR:
#15985
How
Adding request params to Brand and Product campaigns.
Recommended reading order
sponsored_brands.py
sponsored_product.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.