Skip to content

Support Amazon DSP API - Reports entity#57

Merged
denisneuf merged 1 commit into
denisneuf:mainfrom
twistedFantasy:main
Jul 6, 2022
Merged

Support Amazon DSP API - Reports entity#57
denisneuf merged 1 commit into
denisneuf:mainfrom
twistedFantasy:main

Conversation

@twistedFantasy
Copy link
Copy Markdown
Contributor

Initial discussion about this future pull request can be found here
Amazon DSP official documentation can be found here
DSP Reports Swagger API implemented in the scope of this pull request can be found here

Main difference between core Amazon advertising API and DSP is that DSP doesn't use profile_id param which is a required param for most of the other API requests. Also special Accept header should be passed for DSP Report API requests otherwise Not Acceptable error message will be returned. I also removedCredentialProvider from ad_api/base/config.py as it isn't used anywhere and maybe you forgot to delete it during migration to the new version of CredentialProvider located at ad_api/base/credential_provider.py. Will be happy to hear your feedback and fix anything which does not comply with the design convention of this project.

@denisneuf denisneuf merged commit 0291498 into denisneuf:main Jul 6, 2022
@denisneuf
Copy link
Copy Markdown
Owner

Hi Denis, thanks for extend the package and support the DSP reports, would like to check if further release we could pack the access_token_client, client and credential_provider of the specific DSP requirements in the auth and base folder to keep the core of the api inside of that folders.

@twistedFantasy
Copy link
Copy Markdown
Contributor Author

@denisneuf, thank you for a quick review and release! Yes, I also thought about making specific DSP requirements more generic, but wanted to reduce the risk of accidentally breaking other functionality because of the learning curve on this project. But will be happy to review your changes when you have time to implement them.

@denisneuf
Copy link
Copy Markdown
Owner

@twistedFantasy did you try to use the regular credential provider that includes the profile_id ? Do it get you an error if you use the regular client?

@denisneuf
Copy link
Copy Markdown
Owner

denisneuf commented Apr 22, 2023 via email

@denisneuf
Copy link
Copy Markdown
Owner

@drewxa

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants