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 context #581

Merged
merged 5 commits into from
Nov 17, 2021
Merged

Add context #581

merged 5 commits into from
Nov 17, 2021

Conversation

monstermunchkin
Copy link
Member

No description provided.

@stgraber
Copy link
Member

Rather than directly passing the context to effectively every function in the codebase, wouldn't it be easier to add ctx to the relevant structs (ideally common structs when those exist) and then pass/use ctx to functions which support its use?

The checks for whether the context is already done could also likely be pushed onto the caller rather than into every implementation of a source/manager/...

@monstermunchkin
Copy link
Member Author

Rather than directly passing the context to effectively every function in the codebase, wouldn't it be easier to add ctx to the relevant structs (ideally common structs when those exist) and then pass/use ctx to functions which support its use?

Apparently, it's best practice to pass the context directly to a function. I certainly agree, that having it in a struct (where possible) makes life easier. Let's make life easier then :)

@monstermunchkin
Copy link
Member Author

@tomponline
Copy link
Contributor

https://go.dev/blog/context-and-structs

Yeah its ideally supposed to be passed around so that each subsequent operation (HTTP requests, IO operations etc) can then add their own shorter timeouts to it.

That way if you have a chain of operations, they don't all just load the global context, and ignore the context from an earlier operation in the chain.

@stgraber
Copy link
Member

Yeah, interesting to find the right balance.

I think I'd personally start by looking at what function can actually make use of the context, what can be cancelled. That function, if it's public should definitely directly take in a context. Then from there we can check what calls the function and what the lifecycle looks like and decide whether that caller should itself take a context directly (assuming it's also an exported function) or if it can get it from its struct.

So basically exposing the context arg directly through the public functions that we can expect a 3rd party to consume, but we can take shortcuts for our internal usage as we know we won't need to alter the context in those cases and so can use a cheaper way to get it through.

Does that make sense?

@tomponline
Copy link
Contributor

I think I'd personally start by looking at what function can actually make use of the context, what can be cancelled

Absolutely, if a function cannot use a context then there's no point in having it accept an argument for the sake of it.

Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants