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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amazon Seller Partner Fix: 0 records read in reports #9581

Merged
merged 3 commits into from
Jan 19, 2022

Conversation

prudhvi85
Copy link
Contributor

What

All the report streams are returning 0 records

How

source API changed created_since_time to data_start_time

馃毃 User Impact 馃毃

No breaking changes

@github-actions github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Jan 18, 2022
@prudhvi85
Copy link
Contributor Author

@harshithmullapudi @alafanechere. Can you merge this PR? it is kind of urgent for us

@alafanechere alafanechere self-assigned this Jan 18, 2022
@alafanechere alafanechere mentioned this pull request Jan 18, 2022
@alafanechere alafanechere temporarily deployed to more-secrets January 18, 2022 22:09 Inactive
Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. Please provide a clean changelog. I'm running acceptance tests on a side branch, if they're passing I'll publish the new connector version.

docs/integrations/sources/amazon-seller-partner.md Outdated Show resolved Hide resolved
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets January 18, 2022 22:11 Inactive
Co-authored-by: Augustin <augustin@airbyte.io>
@alafanechere
Copy link
Contributor

@prudhvi85 could you explain why this change fixes a problem for you?
According to the API documentation of the currently used version (2020-09-04) the reports endpoint does not take the dataStartTime parameter and createdSince remains valid.
@lizdeika do you encounter the same problem as @prudhvi85?

@prudhvi85
Copy link
Contributor Author

prudhvi85 commented Jan 19, 2022

@alafanechere

Here is the log when we use createdSince as you can see the datastarttime is same as dataendtime. So it is returning 0 records. There is no createdSince In the response from amazon :

{"type": "LOG", "log": {"level": "INFO", "message": "Starting syncing SourceAmazonSellerPartner"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Syncing stream: GET_FLAT_FILE_ALL_ORDERS_DATA_BY_ORDER_DATE_GENERAL "}}
{"type": "LOG", "log": {"level": "WARN", "message": "There are no report document related in stream `GET_FLAT_FILE_ALL_ORDERS_DATA_BY_ORDER_DATE_GENERAL`. Report body {'reportType': 'GET_FLAT_FILE_ALL_ORDERS_DATA_BY_ORDER_DATE_GENERAL', 'processingEndTime': '2022-01-19T09:44:34+00:00', 'processingStatus': 'CANCELLED', 'marketplaceIds': ['ATVPDKIKX0DER'], 'reportId': '710421019011', 'dataEndTime': '2022-01-19T09:44:16+00:00', 'createdTime': '2022-01-19T09:44:16+00:00', 'processingStartTime': '2022-01-19T09:44:22+00:00', 'dataStartTime': '2022-01-19T09:44:16+00:00'}"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Read 0 records from GET_FLAT_FILE_ALL_ORDERS_DATA_BY_ORDER_DATE_GENERAL stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceAmazonSellerPartner"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceAmazonSellerPartner runtimes:\n"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceAmazonSellerPartner"}}

Log when I change createdSince to datastarttime :

{"type": "LOG", "log": {"level": "INFO", "message": "Read 2830 records from GET_FLAT_FILE_ALL_ORDERS_DATA_BY_ORDER_DATE_GENERAL stream"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceAmazonSellerPartner"}}
{"type": "LOG", "log": {"level": "INFO", "message": "SourceAmazonSellerPartner runtimes:\n"}}
{"type": "LOG", "log": {"level": "INFO", "message": "Finished syncing SourceAmazonSellerPartner"}}

@alafanechere
Copy link
Contributor

This doc is not consistent with this one. This is why I'm really confused. I hope this is not a special parameter only valid for en-US...

@prudhvi85
Copy link
Contributor Author

prudhvi85 commented Jan 19, 2022

@alafanechere I tried with Amazon India as well. dataStartTime works fine.

@prudhvi85
Copy link
Contributor Author

@alafanechere I am currently using the local docker image with this fix. Logs from Amazon India. Before the fix, it was returning 0 records.

image

@prudhvi85
Copy link
Contributor Author

Marketplace Amazon India. Connector version - 2.12
image

@alafanechere
Copy link
Contributor

Right @prudhvi85, thank you for testing on the Indian marketplace. I'm publishing the connector now, will merge once it's finish.

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets January 19, 2022 18:35 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets January 19, 2022 18:36 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets January 19, 2022 18:55 Inactive
@alafanechere alafanechere merged commit 48ca806 into airbytehq:master Jan 19, 2022
@alafanechere
Copy link
Contributor

Thanks for this fix @prudhvi85!

@lizdeika
Copy link
Contributor

Hi @prudhvi85 and @alafanechere,

createdSince and dataStartTime are meant for different purposes.

dataStartTime is for data itselft in the report, used when generating report.
createdSince is for report that was created not earlier than YYYY-MM-DD, used when fetching report.

createdSince parameter is meant for getting generated report and not for generating report(like it was implemented until now).
So the implementation was wrong and those reports were not working from the start when someone implemented them.

We do not use those reports so we never noticed.
In my PR I just extracted report data to a method for child classes to be able to extend and/or overwrite it: d7ef91f#diff-7e8be1cd3e8f6950196d15a45ec1df7af0f761f67aa662aef080db5b33b8c4f6R216

@prudhvi85
Copy link
Contributor Author

@lizdeika Now it makes sense. Thanks for the clarification.

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 community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants