-
Notifications
You must be signed in to change notification settings - Fork 0
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
replace intreface with more SDK like interface #1
Conversation
end | ||
|
||
-- service specifics | ||
-- see https://github.com/aws/aws-sdk-js/blob/9295e45fdcda93b62f8c1819e924cdb4fb378199/lib/rds/signer.js#L11-L15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting...so the JS SDK consider this as part of the RDS "service class"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exactly
@Tieske I fixed the param(from self to self.config) that passed to presign function and fixed test accordingly. After the fix I tested locally with my RDS and it worked as expected, thanks for the interface refactor! |
…ance name and database name
@@ -99,7 +99,7 @@ local function getAuthToken(self, opts) --endpoint, region, db_user) | |||
}, | |||
} | |||
|
|||
local presigned_request, err = presign_awsv4_request(self, req_data, opts.signingName, region, RDS_IAM_AUTH_EXPIRE_TIME) | |||
local presigned_request, err = presign_awsv4_request(self.config, req_data, opts.signingName, region, RDS_IAM_AUTH_EXPIRE_TIME) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, sorry about that. 👍
When updating the docs and reviewing the official SDK implementation, I changed the interface to be more like the AWS SDK for JS.
Please have a look. The example code explains how to use it after the changes.
WARNING: I didn't have an RDS database to test against, so please verify it still works
(I did verify the local config changes to work)