🐛 Source Amazon Seller Partner: Fix non vendor accounts connector create/check issue #27050

animer3009 · 2023-06-06T03:27:48Z

What

Bug fix!
After @roman-yermilov-gl change's merge #23980 we got the bug.
Non vendors accounts can't create/check connector.
He changed Orders stream with VendorSalesReports in the check_connection function.
When reviewer asked why did he made this change, he provided this ticket:
#12124

In fact itself @krisjan-oldekamp already did fix early before, just looks like ticket left open...
You can find code by @krisjan-oldekamp :

        # Additional check, since Vendor-ony accounts within Amazon Seller API will not pass the test without this exception
        if "403 Client Error" in str(e):
            return True, None

So ASAP need merge and fix this stuff.

How

With changing Amazon Seller Partner Source connector

github-actions · 2023-06-06T03:28:07Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan and you've followed all steps in the Breaking Changes Checklist
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
You, or an Airbyter, have run /test successfully on this PR - or on a non-forked branch
You've updated the connector's metadata.yaml file (new!)

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

animer3009 · 2023-06-06T03:39:59Z

@lazebnyi @bazarnov @davydov-d @sajarin
Guys please review my changes to merge.

lazebnyi

I requested changes to be made in order to block PR for the purpose of investigating an issue.

roman-yermilov-gl

@animer3009
Could you please provide us with some examples/logs/environment or anything so we can see the bug? The task I refer in my PR is about the same bug but for vendor only accounts, so if we switch back to Orders stream we will switch back to the previous version of this bug. It would be better not to switch between bugs from time to time but figure out the real cause of the issue

animer3009 · 2023-06-06T12:50:50Z

@roman-yermilov-gl we are not switch between bugs!
Please check the code. Issue was already fixed by @krisjan-oldekamp. Just looks like ticket left open. Please check dates as well...
p.s. unfortunately I have the no vendor account...
Can you provide some logs that vendors have issue with old code version?

    # Additional check, since Vendor-ony accounts within Amazon Seller API will not pass the test without this exception
    if "403 Client Error" in str(e):
        return True, None

roman-yermilov-gl · 2023-06-06T14:16:14Z

@roman-yermilov-gl we are not switch between bugs! Please check the code. Issue was already fixed by @krisjan-oldekamp. Just looks like ticket left open. Please check dates as well... p.s. unfortunately I have the no vendor account... Can you provide some logs that vendors have issue with old code version?
    # Additional check, since Vendor-ony accounts within Amazon Seller API will not pass the test without this exception
    if "403 Client Error" in str(e):
        return True, None

Ok, I see the issue #12124 and PR #16629 created by the same person (krisjan-oldekamp) and the issue is few month older than PR. As I also don't have vendor account to check it works, I can only suppose krisjan-oldekamp tested his PR using vendor account because he is a requester of chacnges related to this account type. If so, then the issue was no longer valid at that moment when I started working on it. I agree with changes in this PR

@lazebnyi
It looks like that crime have solved

roman-yermilov-gl · 2023-06-06T18:00:57Z

@animer3009
Could you please clarify what type of bug did you get?
You are saying there is an error handler for vendor only account:

# Additional check, since Vendor-ony accounts within Amazon Seller API will not pass the test without this exception
if "403 Client Error" in str(e):
    return True, None

This error handler works both ways: having Seller account we are using VendorSalesReports stream to check connection. It causes request to fail with 403 error and the error handler (snippet above) should handle it for us properly. The same situation will be for Vendor account and Orders stream.
So it means you PR makes no sense as well as mine #23980 if you will not provide us with information about the bug

animer3009 · 2023-06-06T18:09:27Z

@roman-yermilov-gl
Hmm, theoretically yes, but curious if VendorSalesReports in fact gives 403 Client Error at all...

As I have no vendor account, I am getting error:
HTTPError('400 Client Error: Bad Request for url: https://sellingpartnerapi-na.amazon.com/reports/2021-06-30/reports')

roman-yermilov-gl · 2023-06-06T18:17:15Z

@animer3009
I have just run check_connection using test Sales account and got 403 for VendorSalesReports as expected

roman-yermilov-gl · 2023-06-06T18:19:00Z

@animer3009
I assume there is another cause of 400 Bad Request error on check_connection on your side

animer3009 · 2023-06-06T18:19:33Z

@roman-yermilov-gl
So why it does not work for me?
But in Orders case it is OK?
What if we will have two try there and will check both?

animer3009 · 2023-06-06T18:21:59Z

@roman-yermilov-gl
What can be issue when in Orders case all looks good?
After about 1 hour I will update you.

roman-yermilov-gl · 2023-06-06T18:30:23Z

@animer3009
Is there any other information in error response body? Or maybe you can share for us affected workspace so we can investigate it? I understand that there is something wrong but there may be various reasons: region, wrong advanced stream options, amazon account problems/permissions or whatever else

animer3009 · 2023-06-06T20:13:27Z

@roman-yermilov-gl Sorry but I am not authorized to share my company owned business credentials.

I tried to log it one more time...

2023-06-06 19:37:50 INFO i.a.w.i.VersionedAirbyteStreamFactory(internalLog):312 - I am here!!!!!!
2023-06-06 19:37:50 INFO i.a.w.i.VersionedAirbyteStreamFactory(internalLog):312 - 400 Client Error: Bad Request for url: https://sellingpartnerapi-na.amazon.com/reports/2021-06-30/reports
2023-06-06 19:37:50 INFO i.a.w.i.VersionedAirbyteStreamFactory(internalLog):312 - I am here!!!!!!

Same for the EU with eu account credentials.

2023-06-06 20:10:37 INFO i.a.w.i.VersionedAirbyteStreamFactory(internalLog):312 - I am here!!!!!!
2023-06-06 20:10:37 INFO i.a.w.i.VersionedAirbyteStreamFactory(internalLog):312 - 400 Client Error: Bad Request for url: https://sellingpartnerapi-eu.amazon.com/reports/2021-06-30/reports
2023-06-06 20:10:37 INFO i.a.w.i.VersionedAirbyteStreamFactory(internalLog):312 - I am here!!!!!!

We use this connector in the production...
By rolling back this change (As Vendor's case was already solved) our case will be solved too...
Right now we are only able to create connectors using API as it skips check...

FYI - we have different accounts and we are getting reports from different regions. Just accounts are not Vendor ones and we are facing the same issue for all of them.

Another way is to make two separate checks.
If Orders stream will fail then we will check VendorSalesReports...
This is not the best solution but others also can face this issue and have no idea what to do...
I am just trying to avoid error code dependence with this...

Possibly you are right and somewhere some permission or some other issue is on our account side, but all our all stuff was done by ASP documentations and we have no idea how it can be solved.

animer3009 · 2023-06-06T23:00:03Z

@roman-yermilov-gl
I found the reason!
VendorSalesReports is GET_VENDOR_SALES_REPORT
https://developer-docs.amazon.com/sp-api/docs/report-type-values-analytics

Requests must include the reportPeriod, distributorView, and sellingProgram properties.

It at last need reportPeriod option to be set.

{"GET_VENDOR_SALES_REPORT":{"reportPeriod": "WEEK"}} <- It helped in US region but still have issue for UK...

As users will not have set the option by default, they will also face this issue.

So I am asking merge my change and return back what was before.
Old code handles Vendors case as well.

@lazebnyi please check this stuff.

Thanks!

marcosmarxm · 2023-06-07T11:56:08Z

@roman-yermilov-gl if we return to Orders #12124 won't work... maybe we must find another way to figure out how to do the check connection for Amazon Seller Partner.

animer3009 · 2023-06-07T15:30:46Z

@marcosmarxm It will work...
That was already solved by author...
It was already working before @roman-yermilov-gl changes...

Issue/Ticket time Apr 19, 2022
@krisjan-oldekamp made change and added this lines:

Krisjan O krisjan@gmail.com on 9/26/22 at 1:19 PM

    # Additional check, since Vendor-ony accounts within Amazon Seller API will not pass the test without this exception
    if "403 Client Error" in str(e):
        return True, None

Looks like just issue/ticket left open (was not closed)...

@lazebnyi @marcosmarxm
please check source.py 145 - 149 lines...

roman-yermilov-gl · 2023-06-07T19:23:37Z

@animer3009
In order for this to be merged we need to ensure that everything works right, so we need tests to run successfully on both vendor and seller accounts.

@marcosmarxm I think we need to create vendor account on amazon seller partner with at least one order and then update our acceptance tests by including this account to connection section

marcosmarxm · 2023-06-08T19:02:56Z

@roman-yermilov-gl can you handle this? I don't have much knowledge on Amazon Seller Partner.

roman-yermilov-gl · 2023-06-14T10:21:28Z

@marcosmarxm
I am currently busy with another stuff in scope of Q2B, so I can't spend much time on this issue

cc: @lazebnyi

animer3009 · 2023-06-14T17:23:03Z

@marcosmarxm
@roman-yermilov-gl
@lazebnyi

Can you guys just trust me and merge code lol?
This code was already in "production" for a long time... So we can't make it worse lol :)

animer3009 · 2023-06-21T21:25:43Z

@marcosmarxm
@roman-yermilov-gl
@lazebnyi
Guys,
any idea/any update?

marcosmarxm · 2023-06-27T17:24:52Z

@animer3009 you can build the connector in your side and use the changes. Right now this isn't a priority in our backlog and it will take some time to get finished and merged.

nicksaroki · 2023-07-28T12:45:58Z

@marcosmarxm, could this move up on the team's priority list? It renders the Amazon Selling Partner source as useless. Other complaints about this same issue here: #25674.

marcosmarxm · 2023-07-31T12:28:01Z

@animer3009 @nicksaroki yes, I'm going to handle this week. Amazon Seller Partner tests are broken and I'm working to fix them.

marcosmarxm · 2023-08-02T15:51:33Z

Tests are all green #28492 (comment)

…ate/check issue (#27050) * bux fix. Now non vendor accounts can create/check connection * add bull requestr url * run check for vendor only * update metadata file * remove expectered records from empty stream * add ListFinancialEventGroups as empty stream * remove expected records --------- Co-authored-by: Marcos Marx <marcosmarxm@users.noreply.github.com> Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>

octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Jun 6, 2023

octavia-squidington-iii added community connectors/source/amazon-seller-partner labels Jun 6, 2023

animer3009 added 2 commits June 5, 2023 23:37

bux fix. Now non vendor accounts can create/check connection

07eb029

add bull requestr url

1ebd667

roman-yermilov-gl approved these changes Jun 6, 2023

View reviewed changes

roman-yermilov-gl self-requested a review June 6, 2023 12:00

lazebnyi requested changes Jun 6, 2023

View reviewed changes

roman-yermilov-gl suggested changes Jun 6, 2023

View reviewed changes

roman-yermilov-gl approved these changes Jun 6, 2023

View reviewed changes

animer3009 requested a review from lazebnyi June 6, 2023 16:23

lazebnyi requested a review from marcosmarxm June 13, 2023 17:45

lazebnyi removed their request for review June 22, 2023 15:22

marcosmarxm added the team/tse Technical Support Engineers label Jun 27, 2023

marcosmarxm and others added 6 commits July 19, 2023 17:25

Merge branch 'master' into fix_amazon_seller_partner_connector_check

3f8207a

run check for vendor only

aa5f6e8

Merge branch 'master' into fix_amazon_seller_partner_connector_check

cd8534b

update metadata file

1181e38

Merge branch 'master' into fix_amazon_seller_partner_connector_check

93ad86d

remove expectered records from empty stream

215f74c

marcosmarxm approved these changes Jul 26, 2023

View reviewed changes

marcosmarxm requested review from lazebnyi and removed request for lazebnyi July 26, 2023 18:43

nicksaroki mentioned this pull request Jul 28, 2023

Connector Issue: Seller Partner Source Test Failure #25674

Closed

1 task

lazebnyi approved these changes Jul 31, 2023

View reviewed changes

marcosmarxm and others added 3 commits August 2, 2023 09:28

Merge branch 'master' into fix_amazon_seller_partner_connector_check

da665ca

add ListFinancialEventGroups as empty stream

736f7ee

remove expected records

7e9cb3b

marcosmarxm merged commit 61fa2b6 into airbytehq:master Aug 2, 2023

animer3009 deleted the fix_amazon_seller_partner_connector_check branch August 2, 2023 17:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Source Amazon Seller Partner: Fix non vendor accounts connector create/check issue #27050

🐛 Source Amazon Seller Partner: Fix non vendor accounts connector create/check issue #27050

animer3009 commented Jun 6, 2023 •

edited

Loading

github-actions bot commented Jun 6, 2023

animer3009 commented Jun 6, 2023

lazebnyi left a comment

roman-yermilov-gl left a comment

animer3009 commented Jun 6, 2023 •

edited

Loading

roman-yermilov-gl commented Jun 6, 2023

roman-yermilov-gl commented Jun 6, 2023 •

edited

Loading

animer3009 commented Jun 6, 2023 •

edited

Loading

roman-yermilov-gl commented Jun 6, 2023

roman-yermilov-gl commented Jun 6, 2023

animer3009 commented Jun 6, 2023

animer3009 commented Jun 6, 2023

roman-yermilov-gl commented Jun 6, 2023

animer3009 commented Jun 6, 2023

animer3009 commented Jun 6, 2023 •

edited

Loading

marcosmarxm commented Jun 7, 2023

animer3009 commented Jun 7, 2023 •

edited

Loading

roman-yermilov-gl commented Jun 7, 2023

marcosmarxm commented Jun 8, 2023

roman-yermilov-gl commented Jun 14, 2023

animer3009 commented Jun 14, 2023 •

edited

Loading

animer3009 commented Jun 21, 2023

marcosmarxm commented Jun 27, 2023

nicksaroki commented Jul 28, 2023

marcosmarxm commented Jul 31, 2023

marcosmarxm commented Aug 2, 2023

🐛 Source Amazon Seller Partner: Fix non vendor accounts connector create/check issue #27050

🐛 Source Amazon Seller Partner: Fix non vendor accounts connector create/check issue #27050

Conversation

animer3009 commented Jun 6, 2023 • edited Loading

What

How

github-actions bot commented Jun 6, 2023

Before Merging a Connector Pull Request

animer3009 commented Jun 6, 2023

lazebnyi left a comment

Choose a reason for hiding this comment

roman-yermilov-gl left a comment

Choose a reason for hiding this comment

animer3009 commented Jun 6, 2023 • edited Loading

roman-yermilov-gl commented Jun 6, 2023

roman-yermilov-gl commented Jun 6, 2023 • edited Loading

animer3009 commented Jun 6, 2023 • edited Loading

roman-yermilov-gl commented Jun 6, 2023

roman-yermilov-gl commented Jun 6, 2023

animer3009 commented Jun 6, 2023

animer3009 commented Jun 6, 2023

roman-yermilov-gl commented Jun 6, 2023

animer3009 commented Jun 6, 2023

animer3009 commented Jun 6, 2023 • edited Loading

marcosmarxm commented Jun 7, 2023

animer3009 commented Jun 7, 2023 • edited Loading

roman-yermilov-gl commented Jun 7, 2023

marcosmarxm commented Jun 8, 2023

roman-yermilov-gl commented Jun 14, 2023

animer3009 commented Jun 14, 2023 • edited Loading

animer3009 commented Jun 21, 2023

marcosmarxm commented Jun 27, 2023

nicksaroki commented Jul 28, 2023

marcosmarxm commented Jul 31, 2023

marcosmarxm commented Aug 2, 2023

animer3009 commented Jun 6, 2023 •

edited

Loading

animer3009 commented Jun 6, 2023 •

edited

Loading

roman-yermilov-gl commented Jun 6, 2023 •

edited

Loading

animer3009 commented Jun 6, 2023 •

edited

Loading

animer3009 commented Jun 6, 2023 •

edited

Loading

animer3009 commented Jun 7, 2023 •

edited

Loading

animer3009 commented Jun 14, 2023 •

edited

Loading