Skip to content
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

streaming with Monix's Iterant #1098

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

streaming with Monix's Iterant #1098

wants to merge 1 commit into from

Conversation

alexeygorobets
Copy link

@alexeygorobets alexeygorobets commented Mar 8, 2019 •
edited
Loading

solve #1078

@codecov-io
Copy link

Codecov Report

Merging #1098 into master will decrease coverage by 0.57%.
The diff coverage is 68.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1098      +/-   ##
==========================================
- Coverage   81.04%   80.46%   -0.58%     
==========================================
  Files          54       55       +1     
  Lines        1018     1034      +16     
  Branches       59       55       -4     
==========================================
+ Hits          825      832       +7     
- Misses        193      202       +9
Impacted Files Coverage Δ
monix/src/main/scala/io/finch/monix/package.scala 68.75% <68.75%> (ø)
core/src/main/scala/io/finch/Input.scala 91.8% <0%> (-8.2%) ⬇️
core/src/main/scala/io/finch/Endpoint.scala 73.14% <0%> (+0.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3d374c...799dc54. Read the comment docs.

vkostyukov
vkostyukov reviewed Mar 10, 2019
View reviewed changes
Copy link
Collaborator

@vkostyukov vkostyukov left a comment

Choose a reason for hiding this comment

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

There are in-progress changes (in #1082) that supposed to take advantage of cancellable effects. Depending on which one lands first, we might need to rebase the other one.

build.sbt
@@ -18,6 +18,7 @@ lazy val iterateeVersion = "0.18.0"
lazy val refinedVersion = "0.9.4"
lazy val catsEffectVersion = "1.2.0"
lazy val fs2Version = "1.0.4"
lazy val monixVersion = "3.0.0-RC2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a non-RC version of Monix we can depend on?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like RC2 is the latest. Otherwise we'd have to depend on Monix 2.0 which doesn't have Iterant.

@vkostyukov
Copy link
Collaborator

Hey, @alexeygorobets! Can you rebase your branch and adopt a cancelation logic similar to #1082?

@sergeykolbasov sergeykolbasov mentioned this pull request May 16, 2019
Open
@joroKr21
Copy link
Collaborator

I think this would have to wait for CE3 support in Monix: monix/monix#1533

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rpless rpless rpless left review comments

@vkostyukov vkostyukov vkostyukov left review comments

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@alexeygorobets @codecov-io @vkostyukov @joroKr21 @rpless