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

Add scope to IOLocal #3128

Closed

Conversation

wunderk1nd-e
Copy link

@wunderk1nd-e wunderk1nd-e commented Aug 17, 2022

Thought it could be useful to have a way to update the value of an IOLocal within the scope of some task IO[A]

Motivation was being able to set log context here: typelevel/log4cats#676

@wunderk1nd-e
Copy link
Author

I think the CI failure might be an unrelated issue (flakiness?)

@wunderk1nd-e wunderk1nd-e marked this pull request as ready for review August 23, 2022 09:32
@bplommer
Copy link
Contributor

Could be nice to have this as an instance method on IOLocal?

@iRevive
Copy link
Contributor

iRevive commented Aug 26, 2022

What if we use Resource instead? From my point of view, it offers better flexibility and composability

trait IOLocal[A] {
  def scope[A](value: A): Resource[IO, A] =
    Resource.make(local.getAndSet(value))(local.set)
}

That way, the scope can be managed for Resource, fs2.Stream, and IO:

val local: IOLocal[String] = ???

val stream: fs2.Stream[IO, ...] = fs2.Stream.resource(local.scope("my-scope")).evalMap(_ => ???)
val resource: Resource[IO, ...] = local.scope("external").flatMap(_ => ??? >> local.scope("internal"))
val io: IO[...] = local.scope("my-scope").use(_ => ???)

Copy link
Member

@djspiewak djspiewak left a comment

Choose a reason for hiding this comment

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

Agreed this should probably be Resource, and also would offer better ergonomics as an instance method. Overall though I like this concept!

This was referenced Oct 24, 2022
@djspiewak djspiewak closed this Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants