Skip to content
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

Amazon Ads: add new stream SponsoredDisplayV3ReportStream #47366

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

artem1205
Copy link
Collaborator

@artem1205 artem1205 commented Oct 25, 2024

What

Resolve https://github.com/airbytehq/airbyte-internal-issues/issues/10360


First step in amazon ads enhancement, see full tasklist in https://github.com/airbytehq/airbyte-internal-issues/issues/10429


I decided to leave the old one as is and introduce new one to avoid breaking changes (top-level schema remains the same, but metrics have been changed).
We could remove old stream after bumping the CDK to gracefully wrap non-existing-stream error.

How

add new stream: SponsoredDisplayV3ReportStream

Review guide

  1. airbyte-integrations/connectors/source-amazon-ads/source_amazon_ads/streams/report_streams/display_report.py

User Impact

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

[skip ci]

Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>
@artem1205 artem1205 self-assigned this Oct 25, 2024
Copy link

vercel bot commented Oct 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 25, 2024 0:45am

[skip ci]

Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>
[skip ci]

Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>
Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>
[skip ci]

Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>
Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>
Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>
Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Oct 25, 2024
@artem1205 artem1205 marked this pull request as ready for review October 25, 2024 13:42
@artem1205 artem1205 requested review from maxi297 and a team October 25, 2024 13:45
group_by = ["campaign"]
filters = []

if record_type == "adGroups":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a mapping of values {reportType: (reportTypeId, groupBy, filters)} instead of if...else, and parse it accordingly using get_body() method or similar?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/amazon-ads
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants