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: implement TryParallel operator #221

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

crStiv
Copy link

@crStiv crStiv commented Jan 19, 2025

  • Implements TryParallel struct for parallel execution of operations with error handling
  • Uses futures::try_join! for concurrent execution
  • Adds tests for success and error cases
  • Supports nested parallel operations

Resolves TODO in try_op.rs

@cvauclair
Copy link
Contributor

Hey @crStiv , thanks for the contribution! I had completely forgotten about that TODO 😅

FYI there is already a try_parallel! macro (see this test) which works similarly to the parallel! macro.

I'll still approve this since for regular parallel operations we have both an op Parallel and macro parallel! so it makes sense to have both as well for try_parallel.

Cheers!

@cvauclair cvauclair assigned cvauclair and unassigned cvauclair Jan 20, 2025
@crStiv
Copy link
Author

crStiv commented Jan 20, 2025

Hey @crStiv , thanks for the contribution! I had completely forgotten about that TODO 😅

FYI there is already a try_parallel! macro (see this test) which works similarly to the parallel! macro.

I'll still approve this since for regular parallel operations we have both an op Parallel and macro parallel! so it makes sense to have both as well for try_parallel.

Cheers!

oh, yeah I see now, interesting...

anyway, it was a pleasure to help you!

btw when you'll have time, please merge it

@cvauclair
Copy link
Contributor

@crStiv just need to pass the fmt check (run cargo fmt) and then we can merge, cheers!

@0xMochan
Copy link
Contributor

0xMochan commented Feb 5, 2025

@crStiv it seems like the last commit you've pushed hasn't fixed the cargo fmt issue, can you also rebase w/ main so that this is up to date!

@cvauclair cvauclair added this to the 2025-02-24 milestone Feb 10, 2025
Copy link
Contributor

@0xMochan 0xMochan left a comment

Choose a reason for hiding this comment

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

Looks good!

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.

4 participants