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

Context manager for spawn #1324

Closed
AndreLouisCaron opened this issue Nov 29, 2018 · 5 comments · Fixed by #1732
Closed

Context manager for spawn #1324

AndreLouisCaron opened this issue Nov 29, 2018 · 5 comments · Fixed by #1732
Milestone

Comments

@AndreLouisCaron
Copy link

Hi there!

Thanks for this awesome library, I really boosts my productivity :-)

I've found myself repeating this small context manager in multiple projects, mainly in test suites, to help ensure cleanup is performed properly. I think it might be useful for others as well and I'm wondering if it could be useful to include it in gevent itself as a standard idiom.

@contextmanager
def concurrently(fn, *args, **kwds):
    """Spawn a background task and wait for it to complete."""
    greenlet = gevent.spawn(fn, *args, **kwds)
    try:
        yield greenlet
        greenlet.join()
    finally:
        # NOTE: it's safe to kill even when join() has completed.
        greenlet.kill()

Do you think it could be useful? If so, I could work on a PR :-)

Thanks!

André

@jamadden
Copy link
Member

Thanks for the suggestion! It's not clear to me that this is a widely used pattern, or one that's complex enough to get right that it needs to be in the gevent distribution, rather than a personal utility function. Especially since there are multiple ways to implement it (e.g., subclassing Greenlet to make it itself a context manager and using with Subclass.spawn():) and it doesn't deal with spawn_later, and so on; all of those are various tradeoffs that don't necessarily have clearly "right" answers.

@AndreLouisCaron
Copy link
Author

It's not clear to me that this is a widely used pattern, or one that's complex enough to get right that it needs to be in the gevent distribution, rather than a personal utility function.

There is no good answer to this :-) All "proof" I can offer is that I've copied this recipe across multiple projects now and that I'd prefer having it in a single place rather than copy-pasting it in each new project.

there are multiple ways to implement it (e.g., subclassing Greenlet to make it itself a context manager and using with Subclass.spawn():)

I think that's a question of style preference and I don't really care much which style is used. I just want an idiomatic way to do it.

it doesn't deal with spawn_later, and so on; all of those are various tradeoffs that don't necessarily have clearly "right" answers

I like your suggesting of making Greenlet itself being a context manager because it might be useful precisely for this reason. It would naturally be compatible with all existing ways to spawn a greenlet.

@jamadden
Copy link
Member

It's still not clear to me that it's enough of a win over the current solution of try/finally to warrant an API addition. It seems rare (and unusual) to want to spawn a single greenlet that only runs while some other code is lexically within a single block. More often greenlets are in a pool, or are fire-and-forget, or they're part of a more complicated fan-in/fan-out pattern. Actually, outside of toy examples, I personally can't recall a time when I've wanted a single lexically scoped greenlet.

@AndreLouisCaron
Copy link
Author

I agree that this is not the most common idiom, hence my mention of this being mainly useful in test suites (your "toy examples").

If that's not enough of a convincing use case, here's another.

I write a lot of code where I have a local proxy representation of some remote state. I launch background task that subscribes to change notifications for this remote state. As change notifications come in, the background task updates the local representation and then notifies that changes have occurred using a condition variable (which is why I also submitted issue #1326).

I've written these programs successfully using standard threading and with asyncio, but I vastly prefer gevent's API to both of them (threading doesn't have cancellation support and asyncio is syntactically awful and confusing for many Python devs).

@AndreLouisCaron
Copy link
Author

OMG thanks for fixing this! I'm really looking forward to replacing my custom context manager with the standard one :-)

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 a pull request may close this issue.

2 participants