Cats-Effect intergation (WIP)

[KaranAhlawat]

The PR is in draft, mostly because I'm not sure about

1. If the typeclasses I used in the TF style are correct (I think they should be, but it's better to be 100%)
2. If a Trace implicit is needed by Magnum, and if so, which library should be included in the module.

The tests for now are just the ZIO suite copied over and modified to run with Cats-Effect, and are all passing currently. Tested on Podman.

Please let me know any comments, concerns, advice etc etc you may have!

[AugustNagro]

Thanks for opening this MR @KaranAhlawat !

We just merged some important changes to the ZIO module: #85

I suggest looking at package com.augustnagro.magnum.magzio on the master branch and following the new structure (unless you want something else of course). The big changes are:

1. connect and transact are defined directly on Transactor. For the cats module you may want to add the IO type to Transactor like class Transactor[F: Sync](..). That's how doobie does it and should resolve @BalmungSan 's comment.
2. Removes test dependency on magnum module by copying over the sql resource file we care about.
3. Adds an 'exports' file, that exports some common core magnum types (can copy this from zio module)

[KaranAhlawat]

Hey, thanks, I'll take the changes into account and push some commits

[KaranAhlawat]

Is it fine if I pull in munit-cats-effect as a testing dependency?

[KaranAhlawat]

Okay something might be seriously wrong with the implementation, I'm not entirely sure yet. Please hold off on merging it till I can figure this bug(?) out.

Context: I was using a locally published version to test the API, see how it feels. But for some reason, after a couple runs of the main method, HikariCP throws an exception saying Too many clients already, so I think I borked up the resource cleanup somehow somewhere. Or maybe it's something wrong with my HikariCP configuration, but who knows 😄

Edit: The resource cleanup is working fine, I think it was some other JVM process going Haywire (possibly IntelliJ DB tools)

[AugustNagro]

Is it fine if I pull in munit-cats-effect as a testing dependency?

Yes, absolutely.

[AugustNagro]

Do you see any downside of transact being interruptible @guizmaii ?

I can see the benefit if the user has a request timeout that triggers during a long-running transaction.

[guizmaii]

I don't see any downside. That's even something I might want to backport to the ZIO module 🤔

Note that this interruption is quite brutal: the only way for Cats to interrupt the f call is to call Thread.interrupt, which will stop the thread running the code.
Could this be an issue? 🤔
I don't think so 🤔

[AugustNagro]

Yeah I'd like to see a test where we try to cancel a long running query and see if it actually works.

…

On Sun, Jan 26, 2025, 12:47 AM Jules Ivanic ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In magnum-cats-effect/src/main/scala/com/augustnagro/magnum/magcats/Transactor.scala <#89 (comment)>: > + ) + + def connect[A](f: DbCon ?=> A): F[A] = + useRateLimitedConnection: cn => + Sync[F].delay(connectionConfig(cn)) >> + Sync[F].interruptible(f(using DbCon(cn, sqlLogger))) + + def transact[A](f: DbTx ?=> A): F[A] = + useRateLimitedConnection: cn => + Sync[F] + .delay { + connectionConfig(cn) + cn.setAutoCommit(false) + } >> + Sync[F] + .interruptible(f(using DbTx(cn, sqlLogger))) I don't see any downside. That's even something I might want to backport to the ZIO module 🤔 Note that this interruption is quite brutal: the only way for Cats to interrupt the f call is to call Thread.interrupt, which will stop the thread running the code. Could this be an issue? 🤔 I don't think so 🤔 — Reply to this email directly, view it on GitHub <#89 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQOKNWD7UYAYS6Y4YB2BK32MSOLPAVCNFSM6AAAAABVE67IU2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNZUGE3TGOJWGM> . You are receiving this because you commented.Message ID: ***@***.***>

[KaranAhlawat]

I guess we can add a test for a pg_sleep cancellation, but I'm honestly not sure what that would look like.

[AugustNagro]

I'd be happy to contribute, but it will be a few days.

Like you said we can have a fiber run a query with a long pg_sleep(). Then we interrupt the fiber and make sure that it actually gets cancelled.

My guess is that, since Thread.interrupt just sets a flag, we will need to follow Doobie's example: typelevel/doobie#699