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

add native context support to Run methods #144

Open
carnott-snap opened this issue Feb 16, 2021 · 7 comments
Open

add native context support to Run methods #144

carnott-snap opened this issue Feb 16, 2021 · 7 comments

Comments

@carnott-snap
Copy link

carnott-snap commented Feb 16, 2021

Currently you can manually bind a context.Context to a kong.Context, see #48, but this is not as turnkey as I would have hoped. Would it be possible to add an automatic bind, assuming one is not passed to override, for context.Context that is cancelled when kong.Context.Run returns?

In the override case, I can see arguments for and against, but it may be useful to subcontext via context.WithCancel and explicitly cancel. Need to think through the implications on this one more.

Happy to contribute such a fix, but I thought I would ask if this was wanted first.

@alecthomas
Copy link
Owner

Ah that is not a bad idea. I can't recall if Kong will allow that default binding to be overridden?

@alecthomas
Copy link
Owner

I forgot to say, I would definitely appreciate a PR for this, thanks :)

@falzm
Copy link

falzm commented Jul 4, 2021

@carnott-snap I'd be happy to see this contribution too: I've arrived here via #48 and it's not exactly a very elegant workaround – a native context.Context support would be great 🙏

@alecthomas
Copy link
Owner

Further to this, what would the expected behaviour be if the user subsequently bound a context.Context explicitly?

@falzm
Copy link

falzm commented Sep 28, 2021

A context.Context can be used to pass... well, context-bound values (e.g. metadata) down to the execution function. Although some people strongly disagree with this usage of contexts, it's still supported in the standard library and it'd be nice to leverage this technique in Kong.

@alecthomas
Copy link
Owner

I am aware of what it is... I meant if Kong automatically injects a context.Context, what should occur if the user wants to pass one in? Should Kong not inject its own, error.

@fintronlj
Copy link

I think Kong could add a context to handle OS signals so the application can gracefully shutdown.

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

No branches or pull requests

4 participants