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

Source: Amazon Seller Partner - Updated Report API version + New reports #16629

Merged
merged 9 commits into from
Sep 26, 2022
Merged

Source: Amazon Seller Partner - Updated Report API version + New reports #16629

merged 9 commits into from
Sep 26, 2022

Conversation

krisjan-oldekamp
Copy link
Contributor

@krisjan-oldekamp krisjan-oldekamp commented Sep 13, 2022

What

Fixed #15539

Updated Amazon Seller Reports API verison

Updated version to version 2020-09-04 to 2021-06-30.

Updated connector test

The connector assumed that the "Orders" report is available for every account. So it used the "Orders" endpoint to check if authenticated successfully. However this is not the case for Vendor-only accounts within Amazon Seller API. So did an alternative check.
When you enter invalid credentials, this is the error message:

Exception('Error while refreshing access token: 401 Client Error: Unauthorized for url: https://api.amazon.com/auth/o2/token')"}}

However, when you do have valid credentials, but do not have access to the "Orders" endpoint (since thats not available for Vendor only accounts):

HTTPError('403 Client Error: Forbidden for url: https://sellingpartnerapi-eu.amazon.com/orders/v0/orders?LastUpdatedAfter=xxxx

So I've added an additional check, that checks if "403 Client Error" is in the the error response, and if so, let the connection test pass.

Added new reports

🚨 User Impact 🚨

No. API version update does not alter fieldnames etc.

Community member or Airbyter

  • Community member? Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub by running the /publish command described here
  • After the connector is published, connector added to connector index as described here
  • Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here
Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here
Connector Generator
  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • If adding a new generator, add it to the list of scaffold modules being tested
  • The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
  • Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

@CLAassistant
Copy link

CLAassistant commented Sep 13, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the area/connectors Connector related issues label Sep 13, 2022
@sajarin sajarin changed the title Updated Report API version + New reports Source: Amazon Seller Partner - Updated Report API version + New reports Sep 13, 2022
@sajarin sajarin added the bounty-XL Maintainer program: claimable extra large bounty PR label Sep 13, 2022
@sajarin
Copy link
Contributor

sajarin commented Sep 13, 2022

Hi @krisjan-oldekamp thanks for the PR. This PR is part of Airbyte's Community Maintainer program and someone from our community will be assigned to review your PR and help it get merged. Thanks for being patient and thanks for improving this connector.

@sajarin
Copy link
Contributor

sajarin commented Sep 14, 2022

assigned to @tuanchris

@tuanchris
Copy link
Contributor

Hi @krisjan-oldekamp, can you post the logs of passing integration tests and unit test? I tried running your code locally with Airbyte's credentials and was faced with this error:
403 Client Error: Forbidden for url: https://sellingpartnerapi-na.amazon.com/reports/2021-06-30/reports

@sajarin can you help check with the team to see if they should update the permissions for the new endpionts?

@krisjan-oldekamp
Copy link
Contributor Author

@tuanchris what report are you trying to fetch? The multi-country inventory report is only available in Europe (and I see you're trying to request a report in North America -> https://sellingpartnerapi-**na**.amazon.com/reports/2021-06-30/reports).

amazon-seller-partner-unit-tests.txt

@tuanchris
Copy link
Contributor

@krisjan-oldekamp Oh that can be the case. I'm testing the new streams that you implemented. Can you share the integration tests passing also?

@krisjan-oldekamp
Copy link
Contributor Author

@tuanchris see logs attached (deleted some of the result data).

Good to know, you need to configure these report options, since some of the parameters are mandatory:

{
"GET_VENDOR_INVENTORY_REPORT": {"reportPeriod": "WEEK", "distributorView": "MANUFACTURING", "sellingProgram": "RETAIL"},
"GET_VENDOR_SALES_REPORT": {"reportPeriod": "DAY", "distributorView": "MANUFACTURING", "sellingProgram": "RETAIL"},
"GET_SALES_AND_TRAFFIC_REPORT": {"dateGranularity": "DAY", "asinGranularity": "CHILD"}
}
amazon-seller-partner-integration-tests.txt

I also commited some small changes, as I encountered these issues today.

Copy link
Contributor

@tuanchris tuanchris left a comment

Choose a reason for hiding this comment

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

Hi @krisjan-oldekamp, I left a minor comment below, please have a look. Can you also:

  • run: ./gradlew :airbyte-integrations:connectors:source-amazon-seller-partner:airbytePythonFormat and fix some errors?
  • clean up the codes that you commented out
  • I'm not too familiar with the Amazon seller partner API, can you confirm if bumping API version to 2021-06-30 would require users to reconfigure permissions?

@@ -96,7 +101,7 @@ def get_sts_credentials(config: AmazonSellerPartnerConfig) -> dict:

def check_connection(self, logger: AirbyteLogger, config: Mapping[str, Any]) -> Tuple[bool, Any]:
"""
Check connection to Amazon SP API by requesting the list of reports as this endpoint should be available for any config.
Check connection to Amazon SP API by requesting the list of reports as this endpoint should be available for any config. > NOT THE CASE, for VENDOR-ONLY accounts, the Orders endpoint is not available.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good to know. Please update the doc to describe the new behavior.

@krisjan-oldekamp
Copy link
Contributor Author

@tuanchris

  • I've removed commented out code
  • Updated the connection test doc
  • I've spend a couple of hours to get the Gradle tests to work(....), but I keep getting errors unrelated to the Amazon Seller Partner source, so could you or somebody else run the format test and pass me the things to fix?

@tuanchris
Copy link
Contributor

@krisjan-oldekamp I was able to run it and created a pull request on your branch here https://github.com/stacktonic-com/airbyte/pull/1

@krisjan-oldekamp
Copy link
Contributor Author

@tuanchris Thanks a lot, merged the pull request. And I can confirm users don't have to reconfigure permissions when bumping the API version (I was able to fetch all the reports from the previous and the current version with the same credentials that were created months ago / withouth reconfiguring).

@tuanchris
Copy link
Contributor

Thank you @krisjan-oldekamp
@sajarin good to merge from my end.

@sajarin
Copy link
Contributor

sajarin commented Sep 23, 2022

/test connector=connectors/source-amazon-seller-partner

🕑 connectors/source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/3113967202
✅ connectors/source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/3113967202
Python tests coverage:

Name                                        Stmts   Miss  Cover
---------------------------------------------------------------
source_amazon_seller_partner/spec.py           23      0   100%
source_amazon_seller_partner/__init__.py        2      0   100%
source_amazon_seller_partner/constants.py      37      1    97%
source_amazon_seller_partner/source.py         51     12    76%
source_amazon_seller_partner/streams.py       524    245    53%
source_amazon_seller_partner/auth.py           61     36    41%
---------------------------------------------------------------
TOTAL                                         698    294    58%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       121     25    79%   21-23, 29-31, 36-43, 48-61, 208-216
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       110     48    56%   23-26, 32, 36, 39-64, 67-69, 72-74, 77-79, 82-84, 87-89, 92-110, 144-146
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1322    463    65%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestBasicRead.test_read because not found in the config
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestFullRefresh.test_sequential_reads because not found in the config
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
======================== 22 passed, 3 skipped in 18.47s ========================

@sajarin
Copy link
Contributor

sajarin commented Sep 23, 2022

/publish connector=connectors/source-amazon-seller-partner

🕑 Publishing the following connectors:
connectors/source-amazon-seller-partner
https://github.com/airbytehq/airbyte/actions/runs/3114788952


Connector Did it publish? Were definitions generated?
connectors/source-amazon-seller-partner

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@sajarin
Copy link
Contributor

sajarin commented Sep 23, 2022

@krisjan-oldekamp, not sure I have permission to push to your branch.

Can you bump the version in the Dockerfile and add a line to the changelog in docs/integrations/sources/amazon-seller-partner.md?

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Sep 24, 2022
@krisjan-oldekamp
Copy link
Contributor Author

@sajarin Done!

@sajarin
Copy link
Contributor

sajarin commented Sep 24, 2022

/publish connector=connectors/source-amazon-seller-partner

🕑 Publishing the following connectors:
connectors/source-amazon-seller-partner
https://github.com/airbytehq/airbyte/actions/runs/3117696728


Connector Did it publish? Were definitions generated?
connectors/source-amazon-seller-partner

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@krisjan-oldekamp
Copy link
Contributor Author

@sajarin hmm, access seems to be related to this issue: community/community#5634 I;ve added you and octavia-squidington-iii as a maintainer manually, but not sure if this would work (since status in "pending invite")

@krisjan-oldekamp
Copy link
Contributor Author

@sajarin Is it possible for octavia-squidington-iii to accept the invitation or do you see other ways to fix the access issue?

@sajarin
Copy link
Contributor

sajarin commented Sep 26, 2022

@krisjan-oldekamp, I think it should be good to merge.

@sajarin sajarin merged commit 3939ff3 into airbytehq:master Sep 26, 2022
@sajarin
Copy link
Contributor

sajarin commented Sep 26, 2022

Thanks @krisjan-oldekamp for the PR and @tuanchris for the review!

robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
…rts (airbytehq#16629)

* Updated Report API version + New reports

* Fixed typo and added ability to set fetch period to a fixed range

* Removed old code + updated connection test doc

* minor fix for python format

* Bump version to 0.2.26 and updated docs

* Docs update, fix typo in PR number

Co-authored-by: Tuan Nguyen <anhtuan.nguyen@me.com>
Co-authored-by: Sajarin <sajarindider@gmail.com>
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
…rts (airbytehq#16629)

* Updated Report API version + New reports

* Fixed typo and added ability to set fetch period to a fixed range

* Removed old code + updated connection test doc

* minor fix for python format

* Bump version to 0.2.26 and updated docs

* Docs update, fix typo in PR number

Co-authored-by: Tuan Nguyen <anhtuan.nguyen@me.com>
Co-authored-by: Sajarin <sajarindider@gmail.com>
This pull request was closed.
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 bounty bounty-XL Maintainer program: claimable extra large bounty PR community connectors/source/amazon-seller-partner reward-200
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Source Amazon seller partner: Add support for GET_SALES_AND_TRAFFIC_REPORT
6 participants