Skip to content

parity for Sink, for preMaterialize operation #24657

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 8, 2018
Merged

parity for Sink, for preMaterialize operation #24657

merged 3 commits into from
Mar 8, 2018

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Mar 4, 2018 •
edited
Loading

Resolves #24656

Provides feature parity between preMaterialize on a Sink and a Source.

TODO:

  • add tests
  • add java api

@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR is currently being validated by Jenkins labels Mar 4, 2018
@akka-ci
Copy link

akka-ci commented Mar 4, 2018

Test FAILed.

@ktoso ktoso force-pushed the wip-prematerialize-sink branch from ff2f1c9 to f22854c Compare March 5, 2018 01:28
@ktoso ktoso force-pushed the wip-prematerialize-sink branch from f22854c to ec9796e Compare March 5, 2018 01:30
@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels Mar 5, 2018
@akka-ci
Copy link

akka-ci commented Mar 5, 2018

Test PASSed.

patriknw
patriknw previously requested changes Mar 5, 2018
View reviewed changes
akka-stream/src/main/scala/akka/stream/javadsl/Sink.scala Outdated
*
* Useful for when you need a materialized value of a Sink when handing it out to someone to materialize it for you.
*/
def preMaterialize(materializer: Materializer): japi.Pair[Mat @uncheckedVariance, scaladsl.Sink[In @uncheckedVariance, NotUsed]] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scaladsl.Sink in javadsl

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these uncheckedVariance related to #24575 ?
add java api compile test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, thanks

@ktoso
Copy link
Contributor Author

ktoso commented Mar 7, 2018

Not entirely sure if #24575 resolves the

[error] /Users/ktoso/code/akka/akka-stream/src/main/scala/akka/stream/javadsl/Sink.scala:331:7: contravariant type In occurs in invariant position in type (materializer: akka.stream.Materializer)akka.japi.Pair[Mat @scala.annotation.unchecked.uncheckedVariance,akka.stream.javadsl.Sink[In,akka.NotUsed]] of method preMaterialize
[error]   def preMaterialize(materializer: Materializer): japi.Pair[Mat @uncheckedVariance, Sink[In, NotUsed]] = {

that I had here?

ktoso
ktoso commented Mar 7, 2018
View reviewed changes
Copy link
Contributor Author

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added docs and java test, should be now ready to merge

@ktoso ktoso force-pushed the wip-prematerialize-sink branch from 45b566c to b5a9694 Compare March 7, 2018 04:52
@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed tested PR that was successfully built and tested by Jenkins validating PR is currently being validated by Jenkins labels Mar 7, 2018
@akka-ci
Copy link

akka-ci commented Mar 7, 2018

Test PASSed.

@akka-ci akka-ci added needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed tested PR that was successfully built and tested by Jenkins labels Mar 7, 2018
@akka-ci
Copy link

akka-ci commented Mar 7, 2018

Test FAILed.

@ktoso
Copy link
Contributor Author

ktoso commented Mar 8, 2018

Failure was #24672 it indeed is a bug it seems, we'l have to fix it

@ktoso ktoso dismissed patriknw’s stale review March 8, 2018 02:48

please have a look again

@ktoso
Copy link
Contributor Author

ktoso commented Mar 8, 2018

PLS BUILD

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels Mar 8, 2018
@akka-ci
Copy link

akka-ci commented Mar 8, 2018

Test PASSed.

@ktoso ktoso merged commit 5be3c7b into akka:master Mar 8, 2018
@ktoso ktoso deleted the wip-prematerialize-sink branch March 8, 2018 04:02
@Aaronontheweb Aaronontheweb mentioned this pull request Jun 1, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patriknw patriknw patriknw left review comments

+1 more reviewer

@2m 2m 2m approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ktoso @akka-ci @patriknw @2m