Skip to content

feat: initialize openfunction knative and async runtime - #4

Merged
benjaminhuo merged 7 commits into
OpenFunction:masterfrom
webup:master
Apr 14, 2022
Merged

feat: initialize openfunction knative and async runtime#4
benjaminhuo merged 7 commits into
OpenFunction:masterfrom
webup:master

Conversation

@webup

@webupwebup commented Mar 31, 2022

Copy link
Copy Markdown
Collaborator

This big initial PR is an important upgrade to current GCP Node.js Function Framework with following task accomplished:

  • Add OpenFunction knative runtime
  • Add OpenFunction async runtime
  • Add e2e tests for knative runtime
  • Add e2e tests for async runtime
  • Add conformance test for knative runtime
  • Add conformance test for async runtime
  • Add sequence diagram for knative runtime
  • Add sequence diagram for async runtime
  • Test runtimes in Kubernetes
  • Update auto-generated API docs
  • Refine README

@benjaminhuobenjaminhuo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Big kudos to this PR, just some comments for some corner cases

Comment threadpackage-lock.json Outdated
Comment threadpackage-lock.json Outdated
Comment threadsrc/openfunction/function_runtime.ts
Comment threadsrc/server.ts Outdated
Comment threadsrc/server.ts Outdated
Comment threadsrc/openfunction/dapr_binding_output_middleware.ts Outdated
Comment threadsrc/openfunction/function_runtime.ts
@webup

webup commented Apr 8, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Is it reasonable to make adjustment like below to be consistent with go functions framework?

send(data: object, output?: string)
or
send(output?: string, data: object) ?

In TS, a required param cannot be placed after an optional param.

@benjaminhuo

Copy link
Copy Markdown
Member

Is it reasonable to make adjustment like below to be consistent with go functions framework?

send(data: object, output?: string) or send(output?: string, data: object) ?

In TS, a required param cannot be placed after an optional param.

If the output is not set, where is the data sent to?

@webup

webup commented Apr 8, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Is it reasonable to make adjustment like below to be consistent with go functions framework?

send(data: object, output?: string) or send(output?: string, data: object) ?

In TS, a required param cannot be placed after an optional param.

If the output is not set, where is the data sent to?

Will broadcast to all declared outputs In current impl.

@benjaminhuo

benjaminhuo commented Apr 8, 2022

Copy link
Copy Markdown
Member

The broadcast could be a different method, the send just sends the output to one destination like go implementation maybe?

@webup

webup commented Apr 9, 2022

Copy link
Copy Markdown
CollaboratorAuthor

The broadcast could be a different method, the send just sends the output to one destination like go implementation maybe?

As we discussed in wechat group, let's move on to roll out 0.4.0 first, then will upgrade the function interfaces to better comply with the Function Framework overall guidelines. :)

@webup
webup marked this pull request as ready for review April 10, 2022 09:49
@webupwebup added the type: feature New feature or request label Apr 10, 2022
Comment threadREADME.md
Comment threadREADME.md
@benjaminhuo

Copy link
Copy Markdown
Member

send(data: object, output?: string) seems to be language-specific, there are optional params in nodejs while there isn't in go. We can keep it for now.

@benjaminhuo
benjaminhuo merged commit a9b6224 into OpenFunction:masterApr 14, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: featureNew feature or request

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants

@webup@benjaminhuo