Fix to allow for tokens #70
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As IAM roles also supply a token along with a key and secret, this fix is needed to allow them to connect to DynamoDb.
Nitpick-CI
reviewed
Aug 8, 2017
src/DynamoDbServiceProvider.php
Outdated
| @@ -70,4 +77,4 @@ protected function bindForTesting($marshalerOptions = []) | |||
| return $client; | |||
| }); | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Expected 1 newline at end of file; 0 found
Added new line to end again
baopham
reviewed
Aug 8, 2017
src/DynamoDbServiceProvider.php
Outdated
| @@ -40,11 +40,18 @@ protected function bindForApp($marshalerOptions = []) | |||
| $config = [ | |||
| 'credentials' => [ | |||
| 'key' => config('services.dynamodb.key'), | |||
| 'secret' => config('services.dynamodb.secret'), | |||
| 'secret' => config('services.dynamodb.secret') | |||
There was a problem hiding this comment.
what happen if we just put token here:
'token' => config('services.dynamodb.token')
There was a problem hiding this comment.
It does still work, however I don't believe a parameter we know may or may not be set should be passed through.
I can make the change if you wish, as it doesn't affect functionality.
There was a problem hiding this comment.
I think it's ok. Btw, could you also update README?
There was a problem hiding this comment.
Updated 😃 I've tried to be explicit with the fact the parameter is used for roles.
There was a problem hiding this comment.
Awesome 😄 . Feel free to update the code as discussed:
'token' => config('services.dynamodb.token')There was a problem hiding this comment.
Sorry - misunderstood your comment - code updated!
baopham
reviewed
Aug 8, 2017
src/DynamoDbServiceProvider.php
Outdated
| @@ -41,10 +41,12 @@ protected function bindForApp($marshalerOptions = []) | |||
| 'credentials' => [ | |||
| 'key' => config('services.dynamodb.key'), | |||
| 'secret' => config('services.dynamodb.secret'), | |||
| 'token' => config('services.dynamodb.token') | |||
zoul0813
added a commit
to zoul0813/laravel-dynamodb
that referenced
this pull request
Aug 8, 2017
* master: Allow token for IAM role (baopham#70) Clean up old notes Use Query instead of Scan if conditions contain key Fix composite condition where conditions are not EQ Delete README.v0.3.md Update README.md Fix ModelTrait; ensure we use Eloquent's getTable() as getTableName() does not exist Add FAQ Clean up README
zoul0813
added a commit
to zoul0813/laravel-dynamodb
that referenced
this pull request
Aug 14, 2017
…/laravel-dynamodb into feature/first-find-fail * 'feature/first-find-fail' of https://github.com/zoul0813/laravel-dynamodb: Add firstOrFail, findOrFail support Add build status badge Add limit support (baopham#76) Add .travis.yml Add debug support Clean up Make model serializable Use DateTime::ATOM instead Allow token for IAM role (baopham#70) Clean up old notes Use Query instead of Scan if conditions contain key Fix composite condition where conditions are not EQ Delete README.v0.3.md Update README.md Fix ModelTrait; ensure we use Eloquent's getTable() as getTableName() does not exist Add FAQ Clean up README # Conflicts: # src/DynamoDbQueryBuilder.php # tests/DynamoDbCompositeModelTest.php # tests/DynamoDbModelTest.php
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As using IAM roles, rather than supplying keys directly, you also supply a token along with a key and secret.
If the token isn't supplied, the connection to DynamoDb is unsuccessful. This fix is needed to allow them to connect to DynamoDb.
This change is