-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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 Seller Partner: Add GET_BRAND_ANALYTICS_SEARCH_TERMS_REPORT report #8179
Source Amazon Seller Partner: Add GET_BRAND_ANALYTICS_SEARCH_TERMS_REPORT report #8179
Conversation
…er-GET_BRAND_ANALYTICS_SEARCH_TERMS_REPORT
|
…ANALYTICS_SEARCH_TERMS_REPORT
Hi @lizdeika thanks for your contribution, we'll review it soon! |
…SEARCH_TERMS_REPORT
…ANALYTICS_SEARCH_TERMS_REPORT
Hey @lizdeika what do you think about the above comment ? |
…ANALYTICS_SEARCH_TERMS_REPORT
…ANALYTICS_SEARCH_TERMS_REPORT
@harshithmullapudi I did merge with master and fixed the merge conflicts |
@lizdeika I was more curious on this Hey right now if other reports want to use this same parameter it is bit hard so can we change this to format of
|
@harshithmullapudi Can you guide me here, I do not think I understand what changes should I do 😬 |
Ok it's better if I put this way. What if I want different report_options for different report streams? |
@harshithmullapudi Got it! Please take a look at the changes I've made |
…ANALYTICS_SEARCH_TERMS_REPORT
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.
Just one small change. Otherwise LGTM
...-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/spec.json
Outdated
Show resolved
Hide resolved
def _augmented_data(self, report_options) -> Mapping[str, Any]: | ||
if report_options.get("reportPeriod") is None: | ||
return {} | ||
else: | ||
now = pendulum.now("utc") | ||
if report_options["reportPeriod"] == "DAY": | ||
now = now.subtract(days=1) | ||
data_start_time = now.start_of("day") | ||
data_end_time = now.end_of("day") | ||
elif report_options["reportPeriod"] == "WEEK": | ||
now = now.subtract(weeks=1) | ||
|
||
# According to report api docs | ||
# dataStartTime must be a Sunday and dataEndTime must be the following Saturday | ||
pendulum.week_starts_at(pendulum.SUNDAY) | ||
pendulum.week_ends_at(pendulum.SATURDAY) | ||
|
||
data_start_time = now.start_of("week") | ||
data_end_time = now.end_of("week") | ||
|
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.
can we move this into some util or a static function as this doesn't have any dependency on the class
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.
can we move this into some util or a static function as this doesn't have any dependency on the class
👍🏽 Annotated it as a static but leaving inside the class because the logic is report specific and not reusable
…ANALYTICS_SEARCH_TERMS_REPORT
…ANALYTICS_SEARCH_TERMS_REPORT
@harshithmullapudi I have annotated |
…ANALYTICS_SEARCH_TERMS_REPORT
…PORT report (airbytehq#8179) * Source Amazon Seller Partner: Add GET_BRAND_ANALYTICS_SEARCH_TERMS_REPORT report * loads * fixes and refactors * docs * fixes * Field definitions * versions * versions * refactor report options to scope by REPORT name * fix _report_data * 1 * make _augmented_data a static method * parse_document is static too
What
Adds GET_BRAND_ANALYTICS_SEARCH_TERMS_REPORT report to Amazon Seller Partner source connector
How
Recommended reading order
🚨 User Impact 🚨
Pre-merge Checklist
Expand the relevant checklist and delete the others.
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 here