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

blaze-server on CE3 #4097

Merged
merged 11 commits into from
Jan 2, 2021
Merged

Conversation

RaasAhsan
Copy link

This was fairly straightforward, we just need to think more about where we want to expose the Dispatcher arguments

@RaasAhsan RaasAhsan changed the base branch from main to cats-effect-3 December 28, 2020 20:34
@RaasAhsan
Copy link
Author

ah, we'll need to fix blaze-core up again since fs2 deprecates Queue, which is why the build is failing now

.dequeueChunk1(batchSize)
.map(_.toList)
def pollBatch(batchSize: Int, timeoutSeconds: Long): IO[List[WebSocketFrame]] = {
def batch(acc: List[WebSocketFrame]): IO[List[WebSocketFrame]] =
Copy link
Author

Choose a reason for hiding this comment

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

we should probably add a batched take operation to the CE queue, but it's getting mighty close to fs2 realm

Copy link
Member

Choose a reason for hiding this comment

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

If Queue moved to CE3, then wouldn't the operation?

Copy link
Author

Choose a reason for hiding this comment

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

CE3 implements a brand new Queue separate from fs2.concurrent.Queue. CE3 doesn't have a concept of chunking (this was deemed an fs2 concern), but I think it probably makes sense to add something anyways

Originally the plan was for fs2 Queue to stick around but it seems like Mike made things work out with the CE3 one

Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

Impressive work. Comments below.

.dequeueChunk1(batchSize)
.map(_.toList)
def pollBatch(batchSize: Int, timeoutSeconds: Long): IO[List[WebSocketFrame]] = {
def batch(acc: List[WebSocketFrame]): IO[List[WebSocketFrame]] =
Copy link
Member

Choose a reason for hiding this comment

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

If Queue moved to CE3, then wouldn't the operation?

@RaasAhsan
Copy link
Author

@rossabaker Seems like BlazeBuilder is deprecated in favor of BlazeServerBuilder, should we go ahead and just delete?

@rossabaker
Copy link
Member

Yes, BlazeBuilder can go. It only makes sense to keep deprecations as long as they're source compatible to help people upgrade. That one no longer is, so it's time for it to go.

@RaasAhsan
Copy link
Author

I think everything looks good here now

@rossabaker rossabaker merged commit 4a8040b into http4s:cats-effect-3 Jan 2, 2021
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.

2 participants