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

Implement more flows ops methods part 2 #75

Merged
merged 2 commits into from
Dec 30, 2024

Conversation

emil-bar
Copy link
Contributor

@emil-bar emil-bar commented Dec 19, 2024

Closes #67

@emil-bar emil-bar force-pushed the implement-more-flows-ops-methods-part-2 branch 3 times, most recently from c6e1454 to f6c1fbb Compare December 23, 2024 15:36
* interleave
* mapConcat
* mapPar
* mapParUnordered
* sliding
* alsoTo
* alsoToTap
@emil-bar emil-bar force-pushed the implement-more-flows-ops-methods-part-2 branch from f6c1fbb to a9dc2b6 Compare December 23, 2024 15:38
@emil-bar emil-bar marked this pull request as ready for review December 23, 2024 15:38
@adamw
Copy link
Member

adamw commented Dec 24, 2024

I assume these are directly translated from ox - any places where the implementations diverge?

@emil-bar
Copy link
Contributor Author

emil-bar commented Dec 24, 2024

I assume these are directly translated from ox - any places where the implementations diverge?

Yes, all of them are translated, including tests (I rely on them to see if implementation works).

One additional thing is that for methods that use Scope.unsupervised or Scope.supervised I need to catch ExecutionException and rethrow cause for the tests to pass. See: https://github.com/softwaremill/jox/pull/75/files#diff-46d2537d747d24775cf70630d584ac42326ea6902d3ddd10f5a861841c2c3b6cR738

* remaining non-completed flows.
*/
public static <T> Flow<T> interleaveAll(List<Flow<T>> flows, int segmentSize, boolean eagerComplete, int bufferCapacity) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I must have created this method before adding scope value for buffer capacity. I'll overload the method in the next PR

@adamw
Copy link
Member

adamw commented Dec 30, 2024

One additional thing is that for methods that use Scope.unsupervised or Scope.supervised I need to catch ExecutionException and rethrow cause for the tests to pass. See: https://github.com/softwaremill/jox/pull/75/files#diff-46d2537d747d24775cf70630d584ac42326ea6902d3ddd10f5a861841c2c3b6cR738

Are these tests which verify what exception was thrown? Maybe we could instead change the tests to verify that an EE was thrown?

EE are something that we might want to change in general (see #59), but until then, it would be good to keep consistency.

@emil-bar
Copy link
Contributor Author

One additional thing is that for methods that use Scope.unsupervised or Scope.supervised I need to catch ExecutionException and rethrow cause for the tests to pass. See: https://github.com/softwaremill/jox/pull/75/files#diff-46d2537d747d24775cf70630d584ac42326ea6902d3ddd10f5a861841c2c3b6cR738

Are these tests which verify what exception was thrown? Maybe we could instead change the tests to verify that an EE was thrown?

EE are something that we might want to change in general (see #59), but until then, it would be good to keep consistency.

Yes, those are the tests that verify it. I can fix the tests and verify that EE is thrown. Overall those exceptions are ugly as we are adding 2/3 wrappers on top of source exception in most cases

@adamw
Copy link
Member

adamw commented Dec 30, 2024

@uini223 let's get rid of them in a separate issue then :)

@emil-bar emil-bar force-pushed the implement-more-flows-ops-methods-part-2 branch from 977f78d to 2a4860a Compare December 30, 2024 10:38
@emil-bar
Copy link
Contributor Author

@uini223 let's get rid of them in a separate issue then :)

Removed all rethrows that were present in the code I've added recently

@adamw adamw merged commit 37c1c22 into main Dec 30, 2024
2 checks passed
@adamw adamw deleted the implement-more-flows-ops-methods-part-2 branch December 30, 2024 12:40
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.

implement more flows ops methods - part 2
2 participants