Skip to content

Add FromChannelWithContext() #124

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: v4
Choose a base branch
from
Open

Add FromChannelWithContext() #124

wants to merge 5 commits into from
+68 −3

Conversation

kalaninja
Copy link
Collaborator

resolves #93

@kalaninja kalaninja changed the base branch from master to v4 October 12, 2025 13:49
ahmetb
ahmetb reviewed Oct 12, 2025
View reviewed changes
from.go Outdated

// FromChannelWithTimeout initializes a linq query with a passed channel,
// but stops iterating either when the channel is closed or when the timeout elapses.
func FromChannelWithTimeout[T any](source <-chan T, timeout time.Duration) Query {
Copy link
Owner

Choose a reason for hiding this comment

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

Why don't we just do ctx instead? That way we support any form of cancellation not just time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added FromChannelWithContext method and kept FromChannelWithTimeout for convenience, but I am not sure it is really needed. wdyt?

@kalaninja kalaninja changed the title Add FromChannelWithTimeout() Add FromChannelWithContext() Oct 13, 2025
@kalaninja kalaninja force-pushed the kalaninja/fromchannel-timeout branch from bd0b09e to ae7bc58 Compare October 13, 2025 02:24
ahmetb
ahmetb reviewed Oct 13, 2025
View reviewed changes
@kalaninja kalaninja force-pushed the kalaninja/fromchannel-timeout branch from ee46563 to 1e037ec Compare October 13, 2025 04:40
@kalaninja kalaninja force-pushed the kalaninja/fromchannel-timeout branch from 1e037ec to 771eb7c Compare October 13, 2025 04:44
@kalaninja kalaninja requested a review from ahmetb October 13, 2025 04:45
@ahmetb
Copy link
Owner

ahmetb commented Oct 15, 2025

I think we should just remove the Timeout one. It's fairly easy to implement given ctx signature is offered on another method.

@kalaninja
Copy link
Collaborator Author

kalaninja commented Oct 15, 2025 •
edited
Loading

I think we should just remove the Timeout one. It's fairly easy to implement given ctx signature is offered on another method.

Yes, very much agree, had a similar feeling.

UPDATE: Though we can slightly change FromChannelWithTimeout behavior in a way that might be hard to achieve with FromChannelWithContext. FromChannelWithTimeout can start the timer when iteration starts (opposite to when Query is created). I am not sure if it is needed though.

@ahmetb
Copy link
Owner

ahmetb commented Oct 15, 2025

I think it's fine to just have the ctx version. Typically if someone has a deadline it applies to the whole thing not when we start iterating.

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

Reviewers

@cleitonmarx cleitonmarx Awaiting requested review from cleitonmarx

@ahmetb ahmetb Awaiting requested review from ahmetb

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

how about add parameter timeout for FromChannelT？

2 participants

@kalaninja @ahmetb