-
Notifications
You must be signed in to change notification settings - Fork 49
Prefixed credentials #50
Prefixed credentials #50
Conversation
implicit def fromConfiguration(implicit app: Application): AwsCredentials = { | ||
implicit def fromConfiguration(implicit app: Application): AwsCredentials = fromConfiguration("aws") | ||
|
||
implicit def fromConfiguration(prefix: String = "aws")(implicit app: Application): AwsCredentials = { |
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.
This one does not need to be implicit. I also think the default value is unneeded.
What do you think about:
def fromConfiguration(prefix: String)(implicit app: Application): AwsCredentials
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.
I think you're right about it not needing to be an implicit and the default isn't necessary as well. I'll update the 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.
@EECOLOR
Updated and pushed the new commit into the pull request.
- Moved stuff from play-aws-utils to this library - Refactored WSClient handling, allowing for custom WS clients (fixes kaliber-scala#44) - Moved configuration to it's own class - Refactored policy builder - Added method that allows signing of the url with a different method (fixes kaliber-scala#43)
Yesterday I have worked on the 6.0.0 branch and force pushed it. So it might be a good idea to rebase on that. If you don't know how that works it's fine. As soon as I am able to spend more time I will integrate this change. |
Test to allow for prefixed credentials to be loaded conveninently using `fromConfiguration(prefix)`
Ability to specify multiple credential files in a single configuration then use a prefix passed to `fromConfiguration` to target the appropriate sets. Example: conf: aws.accessKeyId = ... aws.secretKey = ... alt.accessKeyId = ... alt.secretKey = ... calling code: AwsCredentials.fromConfiguration // get aws.X AwsCredentials.fromConfiguration("alt") //get alt.X
After getting feedback from @EECOLOR, removing the implicit keyword from the fromConfiguration method since there isn't a need for it. Also removing the default prefix of "aws", this is done by line 21 in the non-prefixed version of the fromConfiguration method. So we should expect people to specify their prefix's when calling for that functionality specifically.
a393c3b
to
2bbe991
Compare
I've pulled down your force-pushed 6.0.0 branch. Saved my old branch to a different name, branched off of the new 6.0.0 branch, and cherry picked the commits with the prefix credentials feature. Then force pushed my own prefixed-credentials branch so that it shares the common base with your new 6.0.0. |
Awesome. |
d1a04d3
to
91ffb4e
Compare
I have intergrated your commits in the 6.0.0 version |
As originally discussed here
Ability to use multiple credentials in a single application easily.
I can add examples to the README file, but wanted to get feedback on the
implementation itself first.