Skip to content

Conversation

gberzins
Copy link

This pr checks whether request is being made to an endpoint which requires grantless access token.
Endpoints gathered from- https://developer-docs.amazon.com/sp-api/docs/grantless-operations

Relevant issues:
#38
#3

Tested with notification endpoints. Code is not affecting any of generated code parts, so that shouldn't be an issue.

@gberzins
Copy link
Author

gberzins commented Oct 4, 2023

@ericcj pinging you in case you didn't get notification and are interested!

Copy link
Owner

@ericcj ericcj left a comment

Choose a reason for hiding this comment

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

this looks great thank you let's just simplify the config so it doesn't have separate lambdas for different types of tokens and if you could write a bit about how you tested it that would be appreciated i'd like to release a new version including this

config.get_access_token = -> (access_token_key) { Rails.cache.read("SPAPI-TOKEN-#{access_token_key}") }

# optional lambdas for caching grantless LWA access token instead of requesting it each time, e.g.:
config.save_grantless_access_token = -> (access_token_key, token) do
Copy link
Owner

@ericcj ericcj Oct 20, 2023

Choose a reason for hiding this comment

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

the lambdas already take an "access_token_key" which is the cache key to save the token under, let's use the same lambdas just vary the key similar to what i did for RDT https://github.com/ericcj/amz_sp_api/pull/55/files#diff-e2f2fe4083a87c67a20260d6f13385b63f2b64f0cabdcdf4e2f332fa754fbb5bR16

that way we can also remove the indirection of using public_send as well

and after that change the only readme update needed should be to clarify that refresh_token isn't required if you want to use only using grantless operations.

@gberzins
Copy link
Author

@ericcj will do sometime in upcoming week!

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