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

Promise map improved #1775

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

Conversation

mjaix
Copy link

@mjaix mjaix commented Apr 30, 2021

Summary

Generalized Mojo::Promise::Map to also support 'any' and 'all_settled' aggregations plus an optional delay

Motivation

Since the deprecation/removal of Mojo::IOLoop::delay, the only concurrency-limiting elegant way is to use Mojo::Promise::map.
Since I need the limiting as part of a general purpose functionality, I augmented Mojo::Promise::map to support various kinds of aggregation (all[default], any, all_settled) and an optional delay between processing the promises.
Test cases as well as a cookbook entry for Mojo::Promise::map is supplied.

References

LIST RELEVANT ISSUES, PULL REQUESTS AND FORUM DISCUSSIONS HERE

@mjaix
Copy link
Author

mjaix commented May 2, 2021

Ruminating a little bit about not support 'race', I think that should be also done. Albeit 'race' could be simply achieved by applying race() directly to the first n items to be started concurrently, it should be supported in map() to get that function "complete" regarding the support of all possible aggregations.

@mergify
Copy link
Contributor

mergify bot commented Jul 6, 2023

This pull request is now in conflicts. Could you fix it @mjaix? 🙏

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented Aug 14, 2023

This pull request is now in conflicts. Could you fix it @mjaix? 🙏

Copy link
Member

@jhthorsen jhthorsen left a comment

Choose a reason for hiding this comment

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

I know this change is blocked, since it's not rebased, but I wanted to chime in that I'm leaning toward 👎 . I think this is a bit too much to include in core Mojolicious, and I think it could be added as a role on CPAN instead.

Copy link
Contributor

mergify bot commented Mar 6, 2024

This pull request is now in conflicts. Could you fix it @mjaix? 🙏

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.

None yet

3 participants