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

feat: STTP stream #880

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

HollandDM
Copy link
Contributor

@HollandDM HollandDM commented Nov 30, 2024

/claim #764
Will be updated when #896 merged
"Description updating..."

Copy link

algora-pbc bot commented Nov 30, 2024

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe.

@fwbrasil
Copy link
Collaborator

fwbrasil commented Dec 4, 2024

This is looking promising! I tried implementing a Flow Subscriber some time ago but wasn't successful. Thank you for working on it

IO.Unsafe.apply {
val queue = Queue.Unsafe.init[A](capacity = bufferSize, access = Access.MultiProducerMultiConsumer)
val onSubscribePromise = Fiber.Promise.Unsafe.init[Nothing, (Subscription, Queue[A])]()
val kyoSubscriber = new KyoSubscriber[A]:
Copy link
Collaborator

@fwbrasil fwbrasil Dec 4, 2024

Choose a reason for hiding this comment

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

The mix of safe and unsafe code can be tricky as the codebase evolves. It'd be nice to refactor this to a separate class without the AllowUnsafe evidence from IO.Unsafe in scope. Otherwise there's a higher risk of accidentally omitting IO suspensions.

await
} { isEmpty =>
if isEmpty then
_isDone.fold {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind using pattern matching with Absent/Present instead of fold? It's the pattern across the codebase

if isEmpty then
_isDone.fold {
// queue is empty, we parks until notified
awaitPromise.safe.useResult {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this the same as awaitPromise.safe.get?

@fwbrasil
Copy link
Collaborator

fwbrasil commented Dec 4, 2024

I wonder if the new Poll effect could be helpful here #891

@HollandDM
Copy link
Contributor Author

@fwbrasil #896 is a dedicated version of the interop, this PR will be updated to only contains sttp-related changes

@fwbrasil
Copy link
Collaborator

@HollandDM can we close this for now? I wanted to avoid having PRs open for long

@HollandDM
Copy link
Contributor Author

Yeah, lets close it for now, I’ll pick it up later

@HollandDM HollandDM closed this Jan 18, 2025
@fwbrasil fwbrasil reopened this Jan 31, 2025
Copy link

algora-pbc bot commented Jan 31, 2025

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe.

@fwbrasil
Copy link
Collaborator

fwbrasil commented Jan 31, 2025

@HollandDM I'm working on a project that requires streaming. Do you have plans to work on this? I could also collaborate on the implementation to finalize it if you like

@HollandDM
Copy link
Contributor Author

@fwbrasil I’m quite busy these days, so I’m not coming back to this for a while. You can pick up the work if you want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants