Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add flatScheduledTask() methods to EventLoop #1497

Merged
merged 11 commits into from
May 11, 2020
Merged

Add flatScheduledTask() methods to EventLoop #1497

merged 11 commits into from
May 11, 2020

Conversation

gwynne
Copy link
Contributor

@gwynne gwynne commented Apr 29, 2020

Add flatScheduleTask(deadline:) and flatScheduleTask(in:) methods to EventLoop.

Motivation:

Currently the best, or at least simplest, way to schedule a delayed asynchronous task and wait on its complete result is something like eventLoop.scheduleTask(in: ...) { return somethingAsync(...) }.futureResult.flatMap { $0 }. This is not only unobvious but also a bit ugly.

Modifications:

I have added methods which correspond to scheduleTask() in the same way flatSubmit() does to submit(). The returned Scheduled object allows cancellation of the asynchronous task's execution in the same way that is possible for synchronous scheduled tasks, and the futureResult does not complete until the entire asynchronous task does. Callers wishing to wait only for submission of the delayed task, rather than its overall completion, may continue to use the existing scheduleTask() APIs to do so. Tests have been added to verify that the behavior of the new methods is as expected.

I also made a few very minor corrections to a couple of documentation comments which suffered from copy-pasta.

Result:

These changes are purely additive.

gwynne added 2 commits April 29, 2020 07:28
…EventLoop.

Motivation:

Currently the best, or at least simplest, way to schedule a delayed asynchronous task and wait on its complete result is something like `eventLoop.scheduleTask(in: ...) { return somethingAsync(...) }.futureResult.flatMap { $0 }`. This is not only unobvious but also a bit ugly.

Modifications:

I have added methods which correspond to scheduleTask() in the same way flatSubmit() does to submit(). The returned Scheduled<T> object allows cancellation of the asynchronous task's execution in the same way that is possible for synchronous scheduled tasks, and the futureResult does not complete until the entire asynchronous task does. Callers wishing to wait only for submission of the delayed task, rather than its overall completion, may continue to use the existing scheduleTask() APIs to do so. Tests have been added to verify that the behavior of the new methods is as expected.

These changes are purely additive.
@Lukasa Lukasa requested a review from weissi April 29, 2020 12:56
@Lukasa Lukasa added the 🆕 semver/minor Adds new public API. label Apr 29, 2020
@Lukasa Lukasa added this to the 2.17.0 milestone Apr 29, 2020
@weissi weissi requested review from Lukasa and glbrntt April 29, 2020 12:57
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

Thanks very much! This looks good to me (modulo a few nits) :)

Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Thanks; this is helpful! I also have a few nit picks but looks good otherwise.

gwynne added 3 commits April 29, 2020 10:54
…y to precisely control time. For good measure, retrofit the existing tests that were copy/pasted to create the new ones.
@gwynne gwynne requested review from weissi and glbrntt April 29, 2020 15:59
Copy link
Contributor

@glbrntt glbrntt 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 to me!

Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

This looks really good, I only have a few tiny nits! I appreciate that the comment ones aren't introduced by you, but you changed the lines so we may as well clear them up while we're here.

gwynne and others added 2 commits April 30, 2020 04:57
Co-Authored-By: Cory Benfield <lukasa@apple.com>
…seconds(0))`, thus redundant. Fix new tests, original tests, and the bad example in the repeated async task test.
@gwynne gwynne requested a review from Lukasa April 30, 2020 10:04
@gwynne gwynne requested a review from Lukasa April 30, 2020 10:49
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

awesome, thank you!

@Lukasa Lukasa merged commit ac55c57 into apple:master May 11, 2020
pull bot pushed a commit to scope-demo/swift-nio that referenced this pull request May 11, 2020
Motivation:

Currently the best, or at least simplest, way to schedule a delayed asynchronous task and wait on its complete result is something like `eventLoop.scheduleTask(in: ...) { return somethingAsync(...) }.futureResult.flatMap { $0 }`. This is not only unobvious but also a bit ugly.

Modifications:

I have added methods which correspond to scheduleTask() in the same way flatSubmit() does to submit(). The returned Scheduled<T> object allows cancellation of the asynchronous task's execution in the same way that is possible for synchronous scheduled tasks, and the futureResult does not complete until the entire asynchronous task does. Callers wishing to wait only for submission of the delayed task, rather than its overall completion, may continue to use the existing scheduleTask() APIs to do so. Tests have been added to verify that the behavior of the new methods is as expected.

These changes are purely additive.

Co-authored-by: Cory Benfield <lukasa@apple.com>
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
@gwynne gwynne deleted the eventloop-flat-scheduletask branch January 27, 2021 06:15
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants