Skip to content

Changes for refresh amazon app client secret#88

Merged
nitinbhakar merged 8 commits intomasterfrom
INT-5910_Threepn-Automate-the-refresh-of-Amazon-app-client-secret-stage
Jan 24, 2025
Merged

Changes for refresh amazon app client secret#88
nitinbhakar merged 8 commits intomasterfrom
INT-5910_Threepn-Automate-the-refresh-of-Amazon-app-client-secret-stage

Conversation

@nitinbhakar
Copy link
Contributor

@nitinbhakar nitinbhakar commented Jan 21, 2025

muffin_man changes for refresh of amazon app client secret expiry.

@ssontakke
Copy link
Member

Copy link

@Piyush19feb Piyush19feb left a comment

Choose a reason for hiding this comment

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

@nitinbhakar small suggestion.
Please Check

@@ -0,0 +1,20 @@
module MuffinMan::ApiAccountCredential
Copy link
Contributor

@sanjjaymahalingam sanjjaymahalingam Jan 21, 2025

Choose a reason for hiding this comment

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

Maybe this should just be like how the other APIs have been implemented where a sp api client can be created for Application Management API v2023-11-30.

JSON.parse(response.body)["refresh_token"]
end

def self.get_client_credential_refresh_token(client_id, client_secret)

Choose a reason for hiding this comment

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

@nitinbhakar @sanjjaymahalingam I am not sure about this can you check once, in muffin man sp_api_client.rb we have method called retrieve_lwa_access_token will that be useful to generate the refresh_token? and then use for client creation?

Copy link
Contributor Author

@nitinbhakar nitinbhakar Jan 21, 2025

Choose a reason for hiding this comment

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

@rohit-lingayat-sd Yes that can be useful but thats a private method so can't call it directly.
So should i use send() to call private method?

Choose a reason for hiding this comment

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

hmm no, then we can go with your approach then

@@ -0,0 +1,20 @@
module MuffinMan::ApiAccountCredential

Choose a reason for hiding this comment

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

hmm can we have something like this MuffinMan::Credential::V20231130 ?

@@ -0,0 +1,20 @@
module MuffinMan::Credential
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 usually named as mentioned in the SP-API documentation. So this would be MuffinMan::ApplicationManagement

"x-amz-access-token" => "#{access_token}"
}
)
if response.code != 204
Copy link
Contributor

Choose a reason for hiding this comment

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

just returning the response would be better as users of the client can decide further action.

Copy link

@rohit-lingayat-sd rohit-lingayat-sd left a comment

Choose a reason for hiding this comment

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

LGTM!

@nitinbhakar nitinbhakar merged commit 47dbb8a into master Jan 24, 2025
5 checks passed
@nitinbhakar nitinbhakar deleted the INT-5910_Threepn-Automate-the-refresh-of-Amazon-app-client-secret-stage branch January 24, 2025 06:41
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.

5 participants