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

Add basic flow functions and creation methods #63

Merged
merged 6 commits into from
Dec 18, 2024

Conversation

uini223
Copy link
Contributor

@uini223 uini223 commented Dec 10, 2024

No description provided.

  * move from Throwable to Exception
  * add `runFold`, `runDrain`, `onComplete`, `onDone`, `onError`
  * fix error propagation with `buffer` method
  * simplify redundant casting and lambdas
  * fix `groupedWeighted`
  * fix `runLastToChannelAsync`
  * fix `com.softwaremill.jox.FlowEmit.channelToEmit`
  * fix `com.softwaremill.jox.Flows.concat` and  `com.softwaremill.jox.Flows.fromCompletableFuture`
  * migrated more tests
@uini223 uini223 self-assigned this Dec 11, 2024
@uini223 uini223 linked an issue Dec 11, 2024 that may be closed by this pull request
@uini223 uini223 force-pushed the add-basic-support-for-flows branch from 8342557 to a365519 Compare December 12, 2024 10:39
* suppress warnings
* removed redundant exception wrapping in FlowEmit
@uini223 uini223 force-pushed the add-basic-support-for-flows branch from a365519 to 3df33d2 Compare December 12, 2024 11:43
flows/pom.xml Show resolved Hide resolved
@adamw
Copy link
Member

adamw commented Dec 16, 2024

I think this looks relatively complete? If so, mark as ready :)

@uini223 uini223 marked this pull request as ready for review December 16, 2024 11:41
* added module in parent pom
* moved classes into dedicated package
* removed @WithUnsupervisedScope
* <p>
* By default, buffer capacity is unlimited.
* <p>
* Blocks until the flow completes.
Copy link
Member

Choose a reason for hiding this comment

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

this comment is incorrect/lacking:

  1. the channel is rendezvous (capacity 0), not unlimited. Rendezvous channels might mean poor capacity, so I think we should instead by default use a 16-buffered channel (as in Ox). Question is, how to provide this data. Explicit parameters are ... explicit, and not that nice. Scope-local value, maybe? This could be used in other places a swell
  2. the method doesn't block - that's also incorrect in Ox's docs, I'll fix it in a second
  3. there's no mention on why you need a supervision scope - and I think this might not be obvious to users

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. In the next MR, I've added overloaded method with buffer capacity as additional parametr, but there are more methods with similar problems.
    I've added ScopedValue in Channel together with default size.
    I'll also add overloaded method in second MR to keep flexibility.

We might want to consider adding Value Object for buffer capacity to keep things like unlimited buffer, rendevous size, default size, run with scoped etc.

  1. Fixed
  2. Added, please review if is understandable

* <p>
* By default, buffer capacity is unlimited.
* <p>
* Blocks until the flow completes.
Copy link
Member

Choose a reason for hiding this comment

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

this comment is incorrect/lacking:

  1. the channel is rendezvous (capacity 0), not unlimited. Rendezvous channels might mean poor capacity, so I think we should instead by default use a 16-buffered channel (as in Ox). Question is, how to provide this data. Explicit parameters are ... explicit, and not that nice. Scope-local value, maybe? This could be used in other places a swell
  2. the method doesn't block - that's also incorrect in Ox's docs, I'll fix it in a second
  3. there's no mention on why you need a supervision scope - and I think this might not be obvious to users

* fixed documentation
* added ScopedValue support
@uini223 uini223 force-pushed the add-basic-support-for-flows branch from f296448 to d21c53d Compare December 18, 2024 09:36
@adamw adamw merged commit 1c4a4e6 into main Dec 18, 2024
2 checks passed
@adamw adamw deleted the add-basic-support-for-flows branch December 18, 2024 10:01
@adamw
Copy link
Member

adamw commented Dec 18, 2024

Thanks, let's continue on other PRs :)

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.

Add support for Flows
2 participants