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

Activity api #2

Merged
merged 20 commits into from
Apr 25, 2019
Merged

Activity api #2

merged 20 commits into from
Apr 25, 2019

Conversation

maxfischer2781
Copy link
Contributor

Changed the API of rich Activity class to be similar to "bare" activities (aka coroutines).

await activity  # await bare activity

async with Scope() as scope:
    await scope.do(activity) # construct and await rich activity

Key changes invert how to query and wait for completion:

  • await activity.result => await activity returns the result (value or exception) on completion
  • await activity => await acitvity.done delays until completion

Copy link
Member

@eileen-kuehn eileen-kuehn left a comment

Choose a reason for hiding this comment

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

There only seem to be some minor issues with formulations. If those are fixed, we can merge :)

docs/source/glossary.rst Outdated Show resolved Hide resolved
as time passes ("after 20 time units"),
or
at predefined points in time ("at 2000 time units"),

Notification
Information sent to an :term:`activity`, usually in response to an :term:`event`.
Notifications can only be received when an :term:`activity` is :term:`suspended <Suspension>`.
Copy link
Member

Choose a reason for hiding this comment

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

Who can receive the notification? The activity? What about postponed activities?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Being postponed is a special case of being suspended. Should it be made clear that both apply?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, definition of Postponement and Suspension should maybe made more clear. You could put into the description of Postponement that this is a special case of being suspended. This might help.

docs/source/glossary.rst Outdated Show resolved Hide resolved
docs/source/glossary.rst Outdated Show resolved Hide resolved
docs/source/tutorial/04_cancel_scope.rst Outdated Show resolved Hide resolved
usim/_primitives/activity.py Outdated Show resolved Hide resolved
usim/_primitives/activity.py Outdated Show resolved Hide resolved
usim/_primitives/activity.py Outdated Show resolved Hide resolved
usim/_primitives/condition.py Outdated Show resolved Hide resolved
usim/_primitives/condition.py Outdated Show resolved Hide resolved
@maxfischer2781
Copy link
Contributor Author

I have changed the documentation for Activity to better distinguish between the underlying activity (coroutine, awaitable, ...) and the handle for it (an Activity instance). However, it may be better to have an entirely different name.

There is no real consensus in other async frameworks. The asyncio, trio and curio libraries use the term Task for generally the same concept. trio does not seem to allow getting the result or waiting for completion. curio uses Task.join to get the result, and Task.wait to await completion. asyncio only allows waiting for completion using await Task.

Since the Activity class is not meant to be used by-name, we could defer this for now.

@maxfischer2781
Copy link
Contributor Author

The Activity class has been renamed to Task (see issue #3 ), and related classes are renamed as well (e.g. ActivityCancelled => TaskCancelled).

docs/source/glossary.rst Outdated Show resolved Hide resolved
docs/source/tutorial/04_cancel_scope.rst Outdated Show resolved Hide resolved
@eileen-kuehn eileen-kuehn merged commit 3129057 into master Apr 25, 2019
@eileen-kuehn eileen-kuehn deleted the activity-api branch April 25, 2019 20:45
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.

2 participants