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.

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

Make Task.memoize play well with Local #948

Merged
merged 4 commits into from Jul 10, 2019

Conversation

@Avasil
Copy link
Collaborator

commented Jul 9, 2019

Fixes #856

Weird fix but seems to work

@Avasil Avasil requested review from alexandru and oleg-py Jul 9, 2019


val f1 = t.runToFutureOpt
val f2 = t.runToFutureOpt
val f3 = t.runToFutureOpt

This comment has been minimized.

Copy link
@oleg-py

oleg-py Jul 10, 2019

Collaborator

BTW, one of the crazy Scala features is that you can write:
val f1, f2, f3 = t.runToFutureOpt

Not sure if you want to, though 😄

import monix.execution.Scheduler.Implicits.global
implicit val opts = Task.defaultOptions.enableLocalContextPropagation

val task = for {
local <- TaskLocal(0)
v1 <- (local.write(100).flatMap(_ => local.read)).memoizeOnSuccess
v1 <- local.write(100).flatMap(_ => local.read).memoizeOnSuccess

This comment has been minimized.

Copy link
@oleg-py

oleg-py Jul 10, 2019

Collaborator

Just for the sake of inner peace, can you execute the memoized task two or more times to ensure that it doesn't change the local state after first time?

This comment has been minimized.

Copy link
@Avasil

Avasil Jul 10, 2019

Author Collaborator

I upgraded to

val task = for {
    local <- TaskLocal(0)
    memoizeTask = local.write(100).flatMap(_ => local.read).memoize
    v1    <- memoizeTask
    _     <- Task.shift
    v2    <- local.read
    _     <- local.write(200)
    _     <- memoizeTask
    _     <- Task.shift
    v3    <- local.read
} yield (v1, v2, v3)

for (v <- task.runToFutureOpt) yield {
    assertEquals(v, (100, 100, 200))
}

and seems to still work. :D

Initially I changed this test to say that memoize + local don't work, updated scaladoc etc. and then to my surprise the test was failing because it actually still worked... I guess restoreLocals can still be useful if we want to customize locals for Task

@oleg-py
Copy link
Collaborator

left a comment

I didn't even think about that 👍

@Avasil Avasil merged commit 5c63f5b into monix:master Jul 10, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Avasil Avasil deleted the Avasil:local-task-memoize branch Aug 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.