Provide S3 AWS credentials from application.conf #140
Conversation
|
Hi @neowulf33, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement: |
neowulf
force-pushed
the
creds_in_appconf
branch
from
7394e0a
to
362938c
Compare
|
LGTM |
| response <- Http().singleRequest(signedReq) | ||
| _ <- Future(system.log.info("Response: {}", response)) |
There was a problem hiding this comment.
why spawn a future for doing the logging?
what is the toString representation of the request and response? can it be big, and therefore inappropriate for logging?
| val response = for { | ||
| signedReq <- Signer.signedRequest(req, signingKey) | ||
| _ <- Future(system.log.info("Request: {}", signedReq)) |
There was a problem hiding this comment.
instead of using system.log you should use a more specific `LoggingAdapter via
val log = Logging(system, getClass)
|
Oops, those log statements were part of my debugging process. I have removed them. |
| /** | ||
| * | ||
| * @author siva | ||
| */ |
There was a problem hiding this comment.
Please don't use author tags, we encourage collective code ownership: https://github.com/akka/alpakka/blob/master/CONTRIBUTING.md#pull-request-requirements
There was a problem hiding this comment.
Ah my bad. I have removed them in the next commit.
|
LGTM, would like the @author javadoc be removed though as our guidelines require :) |
|
LGTM, thanks a lot! Very nice improvement. |
Fixes #137