Refactor S3 API to static factory methods #1395
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
ennru
reviewed
Dec 21, 2018
There was a problem hiding this comment.
Looks great!
- Do a KazeClass with
S3Settings - Why are there different models in
javadslandscaladsl? They seem to be the same. Check if they may use theWriteMessage/WriteResultnaming standard. - In
HttpRequests: the settings could be passed explicitly
I'll need another look.
s3/src/main/scala/akka/stream/alpakka/s3/impl/MaterializerAccess.scala
Outdated
Show resolved
Hide resolved
s3/src/main/scala/akka/stream/alpakka/s3/impl/MaterializerAccess.scala
Outdated
Show resolved
Hide resolved
s3/src/main/scala/akka/stream/alpakka/s3/impl/HttpRequests.scala
Outdated
Show resolved
Hide resolved
|
Addresses PR feedback. Also updated the way Materializer and Attributes are exposed. Instead of using Materialized value is still wrapped into a Still needs a docs rewrite. |
2m
force-pushed
the
wip-mat-from-gi-2m
branch
2 times, most recently
from
377ff4b to
33f67fc
Compare
2m
changed the title
|
Docs have been refactored. The IBM Bluemix section has been moved to a separate page. Also added a new paragraph and code snipped about using S3Atributes. |
ennru
approved these changes
Jan 14, 2019
|
Fixes #1086 |
|
Fixes #938 |
2m
force-pushed
the
wip-mat-from-gi-2m
branch
from
92e5e42 to
dbe9f50
Compare
|
Addressed feedback and rebased on top of the current master. |
This was referenced Jan 16, 2019
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.
This is a second attempt to clean up S3 connector API. Instead of creating instances of
S3Clientthat then are used to createSource/Flow/Sinks, all of the factories are moved to theS3object. If connector implementation needs an ActorSystem, ExecutionContext or S3Settings, all of those are taken from the materializer.Access to the materialized is factored out to
MaterializerAccess.This PR also enables
MinioS3IntegrationSpecand configuresminiodocker image for that test.Continuation of #1323
Two test cases are failing. Therefore this is still WIP. One testcase
succeed uploading an empty fileis tricky to fix with the current implementation ofMaterializerAccess.source, as empty file (empty source) does not work as other sources withSource.lazily.