-
Notifications
You must be signed in to change notification settings - Fork 8
Pass through session token to signer #11
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
Conversation
| } | ||
|
|
||
| /// Set the session token if credentials given via a task or instance role | ||
| pub fn with_session_token( |
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.
@lpil I have suggestion for better API. I have to check the session token is present later in the call, and before I use this function (given I can run locally with key/pair or on ECS with token)
Perhaps this can be maybe_with_session_token(option.Option(token)) then it's less code in the caller.
Or just with_session_token(option.Option(...))
No preference with me I already wrote the calling code
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.
Having it take an option sounds good to me. Let me know if you want me to merge this or wait for that
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.
ok I change it this afternoon to take option! 👍
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.
@lpil ready for you. Thanks for quick turnaround
(I take any interface you prefer)
lpil
left a comment
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.
Oh very clean! I dig it
77e9076 to
e36974f
Compare
lpil
left a comment
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.
Thank you!
We run this library inside
ECSand therefore we need to pass the session tokenalong with the key-pair.
This change is in production and has been tested