Hey all. I'm migrating a project to blobstore 0.4.1, which also involved migrating our project from fs2 0.x to 1.x.
I notice bufferedPut now requires a ConcurrentEffect and ContextShift as implicit parameters. I'm trying to understand how these are actually used by bufferedPut or a method that it calls. Would anyone be able to explain?
Hi Matt, I don’t have the code in front of me, but cats 1.x introduced ContextShift as a mechanism to wrap a thread pool and allows to run concurrent ops... if I remember correctly bufferedPut would write to disk and then upload to store and it does it with some concurrent streams which would require those constructs
Got it. That makes sense for the most part. I just don't entirely understand how the ContextShift is used in this particular method. I see it as part of the generic type but thats about it. I'm probably missing something obvious here, its been a while since I looked at Scala code
this generic syntax in particular is throwing me off
protected[blobstore] def bufferToDisk[F[_] : ConcurrentEffect : ContextShift]
I'd be in favor of having
remove(path) succeed when the path does not exist. I could make an update to FileStore and whatever other stores are not consistent with this behavior
Curious to hear other people's thoughts as well
would have loved to have #47 in that release,
Path currently behaves unpredicably (from a client’s perspective) with the different schemes
Great to see these contributions! I haven't been watching the repo too closely so feel free to ping here if I haven't acknowledged a PR yet
@matthewgraf @gafiatulin these all look good to me, the last "opinionated" of making all stores regular classes (as opposed to case classes) to hide constructor parameters makes sense to me. Regarding
CS.evalOn changes I'm less familiar with it but assuming this makes it explicitly run in the provided EC which sounds safer to me. Maybe @Daenyth @Igosuki @DougC have a stronger position on any of these changes since they have worked more on the upgrades to newer scala/cats/fs2 versions, but otherwise I'm good with these changes.
it's absolutely necessary for healthy thread pool usage to use a blocking EC correctly; the recommended cats-effect setup (and the way that the ecosystem works) is that you use a fixed-size (= cores) thread pool as your primary EC that backs your
ContextShift, and a cached thread pool for jvm-thread-blocking calls, using evalOn to make sure those calls happen on the thread blocking pool
if you do thread-blocking calls without using
evalOn (or related) to put it on the right pool, you risk severely hampering the application's efficiency
If you would like more maintainers, @gafiatulin and I volunteer
Hi @jgogstad sorry for the delay, its been hard to sync up since Matt is currently the only one with merge access and access to LendUp's GPG keys... I'll try to sync up with him and get this merged and published ASAP. We will also be moving this to a separate GitHub org so we can open it to more maintainers. Once we merge and publish these last PRs we will go ahead and do that.
ah. looks like an easy fix
Sorry again for the delay. Ditto what Rolando said about moving this to another org, we need more maintainers besides me :sweat:
@gafiatulin would you mind pulling master into your branch? lendup/fs2-blobstore#49
Looks like Travis is using the travis.yml from your branch, not master. I thought it would merge master first and use travis.yml from there. Just trying to get a green build before merging
@jgogstad can you fix conflicts on lendup/fs2-blobstore#50 ? I will merge this today. Trying to get this one in and #52 before doing a release
also @gafiatulin for lendup/fs2-blobstore#52
they might be straightforward but you probably have more context
they might be straightforward but you probably have more context