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 defining variables and methods and fix some issues #52

Merged
merged 4 commits into from
Jun 28, 2022

Conversation

lizzzcai
Copy link
Member

@lizzzcai lizzzcai commented Jun 18, 2022

  1. use mux as the default router to support defining variables (pass by context) and methods (not supported in cloudevent as other methods are disabled by default in the current implementation).
  2. every function has its own function context in the multi-function use case (functioname -> function context), otherwise, the function will read the result of others
  3. in openfunction function type, pass the userdata to the function if the request is a cloud event, which matches the behavior in async.
  4. fix some small issues.

find example here

close #51

Signed-off-by: Lize Cai lize.cai@sap.com

…fine some features. fix some issues. add tests.

close OpenFunction#51

Signed-off-by: Lize Cai <lize.cai@sap.com>
Signed-off-by: Lize Cai <lize.cai@sap.com>
defer RecoverPanicHTTP(w, "Function panic")
rm.FunctionRunWrapperWithHooks(rf.GetOpenFunctionFunction())

switch rm.FuncOut.GetCode() {
case ofctx.Success:
w.Header().Set(functionStatusHeader, successStatus)
w.WriteHeader(rm.FuncOut.GetCode())
w.Write(rm.FuncOut.GetData())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tpiperatgod do we need to w.Write(rm.FuncOut.GetData()) here?
we don't have this previously

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to refer to the mechanism of the ResponseWriter in the http package: https://pkg.go.dev/net/http#ResponseWriter

runtime/runtime.go Outdated Show resolved Hide resolved
runtime/knative/knative.go Outdated Show resolved Hide resolved
runtime/knative/knative.go Outdated Show resolved Hide resolved
runtime/runtime.go Show resolved Hide resolved
framework/framework.go Outdated Show resolved Hide resolved
@benjaminhuo
Copy link
Member

This is a big enhancement to current go ff.
Can we add more samples into the samples repo and docs to the openfunction.dev site?
@lizzzcai

@lizzzcai
Copy link
Member Author

This is a big enhancement to current go ff. Can we add more samples into the samples repo and docs to the openfunction.dev site? @lizzzcai

Sure. I will add samples and update the docs in other PRs.

I will update the code with the above comments later.

For the function context, as multi user functions feature is only in knative runtime now which don't have input binding and pubsub. For the output bindings, we will leave it open for all the user functions so the user can access all the output binding in ctx.send(...).

However, for the plugins, I am not sure how to handle it now. As the plugin is in the framework level, it will apply to all user functions.
I can come up with two use cases,

  1. metrics, tracing etc. Which is good to be in framework level and apply in all the user functions. (for example, skywalking)
  2. pre and post processing of a user function. I think for this use case, it is not so necessary to use plugin, as I can do it in the user function as well by wrapping the function.

maybe we need to clearly define the usecase of the plugins.

cc @tpiperatgod

@benjaminhuo
Copy link
Member

benjaminhuo commented Jun 24, 2022

However, for the plugins, I am not sure how to handle it now. As the plugin is in the framework level, it will apply to all user functions. I can come up with two use cases,

  1. metrics, tracing etc. Which is good to be in framework level and apply in all the user functions. (for example, skywalking)
  2. pre and post processing of a user function. I think for this use case, it is not so necessary to use plugin, as I can do it in the user function as well by wrapping the function.

maybe we need to clearly define the usecase of the plugins.

We can use annotations in function's crd to provide additional context for plugins as we discussed in yesterday's meeting
And this can be added in another proposal and PR

…oud event

Signed-off-by: Lize Cai <lize.cai@sap.com>
@benjaminhuo benjaminhuo merged commit d08c60f into OpenFunction:main Jun 28, 2022
@benjaminhuo
Copy link
Member

Thanks for the PR!

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 this pull request may close these issues.

[Feature]Support advanced http router and features
3 participants