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

Support context propagation on overloads #557

Open
1 of 3 tasks
nlachfr opened this issue Jun 26, 2022 · 3 comments · May be fixed by #558
Open
1 of 3 tasks

Support context propagation on overloads #557

nlachfr opened this issue Jun 26, 2022 · 3 comments · May be fixed by #558

Comments

@nlachfr
Copy link
Contributor

nlachfr commented Jun 26, 2022

Feature request checklist

  • There are no issues that match the desired change
  • The change is large enough it can't be addressed with a simple Pull Request
  • If this is a bug, please file a Bug Report.

Change

Currently, it is possible to use go contexts with the ContextEval(context.Context, interface{}) method. However, this context is not usable by function overloads.

Example

fn := &functions.ContextOverload{
    Operator: "longRunningOperation",
    Unary: func(ctx context.Context, value ref.Val) {
        if err := longRunningOperation(ctx, value.Value()); err != nil {
            return types.Bool(false)
        }
        return types.Bool(true)
    }
}
@nlachfr nlachfr linked a pull request Jun 26, 2022 that will close this issue
@TristonianJones
Copy link
Collaborator

This is similar to what's exposed in #368; however, in that PR the Context argument is not stored. Also, in Kubernetes it seems to be more common to store the callback channel.

There's been interest in reviving #368, so I'll take a look at what you're proposing and hopefully merge the approaches as best I can

@odsod
Copy link

odsod commented Nov 22, 2022

@TristonianJones Is this still on the agenda? We might be a bit out of line doing async calls as part of custom CEL functions, but this would simplify those use cases quite a bit (partly in terms of propagating request deadline, but also caller authentication for upstream RPCs!).

@nlachfr
Copy link
Contributor Author

nlachfr commented Jan 12, 2023

@TristonianJones How can I help in adding this feature ?
For now, #368 seems to be frozen and being able to use the execution Context in overloads will be useful for network related functions

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.

3 participants