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

ObjMethod: Always pass context.Context for API method calls #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SimonRichardson
Copy link
Member

All API server calls make passing context.Context optional. This means that most if not all API requests do not utilise the context available to them. This means that it makes it a lot hard to know when a request should be cancelled or discarded.

This will become really useful when migrating a way from mongo and to another database store. Additionally we can start to hang more things from the context allow them to pass through the stack.

This is a breaking change, by forcing context.Context to become non-optional and forcing juju to handle context correctly.

All API server calls make passing context.Context optional. This means
that most if not all API requests do not utilise the context available
to them. This means that it makes it a lot hard to know when a request
should be cancelled or discarded.

This will become really useful when migrating a way from mongo and to
another database store. Additionally we can start to hang more things
from the context allow them to pass through the stack.

This is a breaking change, by forcing context.Context to become
non-optional and forcing juju to handle context correctly.
SimonRichardson added a commit that referenced this pull request Feb 9, 2023
@jameinel
Copy link
Member

jameinel commented Feb 9, 2023

I'm in favor of this, though I wonder if we are better off having context.Context mean something on the juju side. Otherwise you have the idea that it is 'cancelable' but nobody is actually listening on the other side.
Can we tie the context.Context to the apiserver's tomb that already handles teardown/cancelation ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants