Add NonEmptyHotswap by morgen-peschke · Pull Request #4267 · typelevel/cats-effect

morgen-peschke · 2025-02-06T19:25:29Z

Also corrects the site docs for Hotswap so it conforms to the Hotswap API

Addresses #4268

armanbilge

Thanks for the PR!

Honestly, I wonder if we should deprecate the old HotSwap.

Hotswap[F, A] is just NonEmptyHotswap[F, Option[A]]. We don't need both.
The current Hotswap API is broken anyway, e.g. see #3480 (comment).

@djspiewak I know you hate it, but there are so many things we could fix if we just embrace the ugly Hotswap2, Queue2, 😅

Oh, also you should retarget to series/3.6.x if you want it to make the next release (@djspiewak are we still merging new APIs into this branch?)

std/shared/src/main/scala/cats/effect/std/NonEmptyHotswap.scala

std/shared/src/main/scala/cats/effect/std/Hotswap.scala

Thanks to armanbilge for the [suggestion](typelevel#4267 (comment))

kamilkloch · 2025-02-19T10:40:21Z

@djspiewak I know you hate it, but there are so many things we could fix if we just embrace the ugly Hotswap2, Queue2, 😅

sttp has embraced this path a long time ago and it works like a charm :) https://github.com/softwaremill/sttp/tree/master/core/src/main

morgen-peschke · 2025-04-07T17:56:39Z

@armanbilge did you want the Hotswap2 rename?

armanbilge · 2025-04-07T17:58:10Z

I do, but I'm not sure if @djspiewak has bought in.

morgen-peschke · 2025-04-24T19:56:01Z

@djspiewak could we get a ping if this is something you'd like to weigh in on?

morgen-peschke · 2025-05-15T22:40:32Z

@armanbilge it's been 6 weeks without a ping from @djspiewak, can we take that as "decline to comment" and proceed?

morgen-peschke · 2025-06-12T21:37:48Z

@armanbilge no shade intended by this question, just want to know if I should close it: is this a dead PR?

armanbilge · 2025-06-13T16:56:50Z

@morgen-peschke review and release cycles in Cats Effect are often very slow, sorry about that, and thanks for your patience ☺️

morgen-peschke · 2025-06-13T17:10:29Z

@armanbilge what's the culturally acceptable frequency of comment-bumping a PR in this state?

I'd really like to offload having to remember to check back on this to a repeating calendar event and I also don't want to be a bother by posting too often just to keep the PR from sliding completely off the radar, if the expectation is that this could be another 3+ months.

ETA: In case the above came across wrong, I want to reiterate that no slight is intended, I'm trying to figure out the best way to manage the time and energy I'm going to spend keeping track of this PR, as well as avoid creating the association in y'all's mind as that annoying dev who won't shut up about that one PR.

armanbilge

I spoke to Daniel offline, he is alright with moving ahead with Hotswap2. Thanks for your patience!

I just noticed ... we will need to retarget this PR at series/3.x and I think this will require rewriting the test suite using munit, sorry about that!

armanbilge · 2025-07-10T21:20:39Z

docs/std/hotswap.md

+## Hotswap
+
+In cases where the managed `Resource` may be missing, `Hotswap` provides some optimizations for this use case. 


Sorry if I'm missing something, what is this sentence referring to exactly?

It's referring to some of the stuff that Hotswap does around avoiding blocking swap after a call to get returns a Resource containing a None.

If we're fully deprecating Hotswap, we can probably drop this section entirely.

armanbilge · 2025-07-10T21:21:09Z

std/shared/src/main/scala/cats/effect/std/Hotswap.scala

I think we can fully deprecate Hotswap in favor of Hotswap2.

armanbilge · 2025-07-10T21:22:11Z

std/shared/src/main/scala/cats/effect/std/Hotswap.scala

+    Hotswap2.create[F, R].map { nes =>
+      new Hotswap[F, R] {
+        override def swap(next: Resource[F, R]): F[R] =
+          nes.swap(next.map(_.some)) *> get.use(_.get.pure[F])


This is the "leaking" implementation, correct? Maybe we should add a comment saying this is intentional, to satisfy the mistakes of the old api.

std/shared/src/main/scala/cats/effect/std/Hotswap2.scala

armanbilge · 2025-07-10T21:32:48Z

std/shared/src/main/scala/cats/effect/std/Hotswap2.scala

+   * [[swap]] finalizes the previous resource immediately, so users must ensure that the old `R`
+   * is not used thereafter. Failure to do so may result in an error on the _consumer_ side. In
+   * any case, no resources will be leaked.
+   *
+   * For safer access to the current resource see [[get]], which guarantees that it will not be
+   * released while it is being used.


These docs can be updated, they are trying to massage around the shortcomings of the old API.

Squash - Deprecate Hotswap - Apply suggestions from code review Co-authored-by: Arman Bilge <armanbilge@gmail.com> - Remove unsafe returned value from swap - prePR - Implement Hotswap in terms of NonEmptyHotswap - Also adds two methods to make `NonEmptyHotswap` a drop-in replacement for `Hotswap` - Revert "Implement Hotswap in terms of NonEmptyHotswap" - This reverts commit 8dc9fcf. - Add a not about the differences between Hotspot and NonEmptyHotspot - Revert "Add a not about the differences between Hotspot and NonEmptyHotspot" - This reverts commit bcfb4b0. - Revert "Revert "Implement Hotswap in terms of NonEmptyHotswap"" - This reverts commit 4b7ebe4. - Implement lock aquiring shenanigans and restore related test Thanks to armanbilge for the [suggestion](typelevel#4267 (comment)) - Switch NonEmptyHotswap to Hotswap2 - Apply suggestions from code review Co-authored-by: Arman Bilge <armanbilge@gmail.com> - Deprecate Hotswap - Documentation fixes in Hotswap2 - Add Hotswap2.getOpt This would allow upgrading from Hotswap without losing the semantics of not acquiring a lock when no resource is present for `Hotswap2[F, Resource[Option[R]]]`

Apparently, that day was not today

morgen-peschke · 2025-07-12T20:46:13Z

@armanbilge the test failure appears to be unrelated to this change, if everything else looks good, can you retrigger CI? If something needs changed, don't worry about it, since it should clear on the next push 🤞🏻

std/shared/src/main/scala/cats/effect/std/Hotswap.scala

djspiewak

Is there a reason we moved away from NonEmptyHotswap as a name? Aesthetically that seems better to me than Hotswap2. We could still deprecate Hotswap even in that world.

morgen-peschke · 2025-07-23T15:58:53Z

Is there a reason we moved away from NonEmptyHotswap as a name? Aesthetically that seems better to me than Hotswap2. We could still deprecate Hotswap even in that world.

This isn't something I have strong opinions about, so you'll have to check in with @armanbilge for the reasons behind it.

Context: originally raised in this comment, I swapped the names because it was requested by the primary reviewer, I couldn't think of a good reason not to do it, and I didn't want to hold up the PR.

armanbilge · 2025-07-23T16:10:38Z

@djspiewak

Aesthetically that seems better to me than Hotswap2.

I don't want to hold this up over aesthetic concerns 😁 if NonEmptyHotswap feels better, let's just do that. To be clear, I still feel strongly that the original Hotswap should be deprecated.

The reason I suggested Hotswap2 is because I think this new API should be Hotswap, not just a different flavor of it. The original Hotswap API is broken, and my personal opinion is the "emptiness" of it is also kind of weird. So describing this new API as the non-empty version of this broken, deprecated API did not sit right with me.

std/shared/src/main/scala/cats/effect/std/Hotswap.scala

I think everything is addressed?

morgen-peschke mentioned this pull request Feb 6, 2025

Site Doc for Hotswap has a broken example #4268

Closed

armanbilge reviewed Feb 7, 2025

View reviewed changes

morgen-peschke changed the base branch from series/3.x to series/3.6.x February 7, 2025 17:14

morgen-peschke force-pushed the add-nonempty-version-of-hotswap branch from 11ca512 to e9965a3 Compare February 7, 2025 17:23

armanbilge reviewed Feb 7, 2025

View reviewed changes

std/shared/src/main/scala/cats/effect/std/Hotswap.scala Outdated Show resolved Hide resolved

morgen-peschke added a commit to morgen-peschke/cats-effect that referenced this pull request Feb 8, 2025

Implement lock aquiring shenanigans and restore related test

a4a6bda

Thanks to armanbilge for the [suggestion](typelevel#4267 (comment))

armanbilge previously requested changes Jul 10, 2025

View reviewed changes

armanbilge added the 🍄 enhancement label Jul 10, 2025

morgen-peschke force-pushed the add-nonempty-version-of-hotswap branch from a49624e to bfadf2b Compare July 12, 2025 18:33

morgen-peschke force-pushed the add-nonempty-version-of-hotswap branch from bfadf2b to e114c7d Compare July 12, 2025 18:36

morgen-peschke changed the base branch from series/3.6.x to series/3.x July 12, 2025 18:37

morgen-peschke force-pushed the add-nonempty-version-of-hotswap branch from 87b5ce7 to e8f3003 Compare July 12, 2025 18:39

morgen-peschke added 4 commits July 12, 2025 12:04

Rewrite Hotswap2 tests to MUnit

764c50f

Add deprecation to tests to silence warning

51041dc

Scalafix

a5eb746

One day I will remember that ScalaDoc doesn't understand import

0bac511

Apparently, that day was not today

morgen-peschke requested a review from armanbilge July 12, 2025 20:45

armanbilge reviewed Jul 16, 2025

View reviewed changes

std/shared/src/main/scala/cats/effect/std/Hotswap.scala Outdated Show resolved Hide resolved

Update deprecation notice with correct version

47b0afa

djspiewak reviewed Jul 23, 2025

View reviewed changes

Switch back to NonEmptyHotswap

93cf267

morgen-peschke requested review from armanbilge and djspiewak July 23, 2025 17:06

mzuehlke reviewed Jul 23, 2025

View reviewed changes

std/shared/src/main/scala/cats/effect/std/Hotswap.scala Outdated Show resolved Hide resolved

Missed a few renames

9a32ef0

morgen-peschke requested a review from mzuehlke July 23, 2025 17:38

djspiewak approved these changes Jul 23, 2025

View reviewed changes

mzuehlke approved these changes Jul 23, 2025

View reviewed changes

djspiewak merged commit 160c1c6 into typelevel:series/3.x Jul 23, 2025
36 of 38 checks passed

morgen-peschke deleted the add-nonempty-version-of-hotswap branch July 23, 2025 23:34

		## Hotswap

		In cases where the managed `Resource` may be missing, `Hotswap` provides some optimizations for this use case.

Uh oh!

Conversation

morgen-peschke commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

armanbilge left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kamilkloch commented Feb 19, 2025

Uh oh!

morgen-peschke commented Apr 7, 2025

Uh oh!

armanbilge commented Apr 7, 2025

Uh oh!

morgen-peschke commented Apr 24, 2025

Uh oh!

morgen-peschke commented May 15, 2025

Uh oh!

morgen-peschke commented Jun 12, 2025

Uh oh!

armanbilge commented Jun 13, 2025

Uh oh!

morgen-peschke commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

armanbilge left a comment

Choose a reason for hiding this comment

Uh oh!

armanbilge Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

morgen-peschke Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

armanbilge Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

armanbilge Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

armanbilge Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

morgen-peschke commented Jul 12, 2025

Uh oh!

Uh oh!

djspiewak left a comment

Choose a reason for hiding this comment

Uh oh!

morgen-peschke commented Jul 23, 2025

Uh oh!

armanbilge commented Jul 23, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

morgen-peschke commented Feb 6, 2025 •

edited

Loading

armanbilge left a comment •

edited

Loading

morgen-peschke commented Jun 13, 2025 •

edited

Loading