Skip to content

feat: init binding and async #1

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

Merged

Conversation

kehuili
Copy link
Collaborator

@kehuili kehuili commented May 11, 2022

This big initial PR is an important upgrade to current GCP Python 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 sequence diagram for knative runtime
  • Add sequence diagram for async runtime
  • Refine README

@kehuili kehuili added the enhancement New feature or request label May 11, 2022
@kehuili kehuili requested review from webup and benjaminhuo May 11, 2022 12:47
anniefu and others added 2 commits May 11, 2022 20:48
Fix failing assert to look for a different error message, likely due to
a change in CloudEvents error message.

Also add tests so that code coverage is back at 100%.

Signed-off-by: Kehui Li <kehui.li@uisee.com>
Signed-off-by: Kehui Li <kehui.li@uisee.com>
@CLAassistant
Copy link

CLAassistant commented May 11, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ Kehui Li
❌ anniefu


Kehui Li seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@kehuili kehuili force-pushed the feature/init_binding_and_async branch from de96a02 to 05df573 Compare May 11, 2022 12:48
@benjaminhuo
Copy link
Member

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.0 out of 2 committers have signed the CLA.❌ Kehui Li❌ anniefu

Kehui Li seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

That's a great enhancement! Thanks @kehuili
Would you sign the CLA? You might need to get around the GFW to sign it.

And to utilize this python functions-framework, we might need to update https://github.com/OpenFunction/builder as well.
Let's discuss this in today's meeting.

Ben

@kehuili
Copy link
Collaborator Author

kehuili commented May 12, 2022

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.0 out of 2 committers have signed the CLA.❌ Kehui Li❌ anniefu
Kehui Li seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

That's a great enhancement! Thanks @kehuili Would you sign the CLA? You might need to get around the GFW to sign it.

And to utilize this python functions-framework, we might need to update https://github.com/OpenFunction/builder as well. Let's discuss this in today's meeting.

Ben

Just signed CLA. Also I picked a commit from GCP for a test fix (or one unit test would fail forever), it seems he/she also need to sign. Do I need to revert this commit and just commit this change myself?

@benjaminhuo
Copy link
Member

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.0 out of 2 committers have signed the CLA.❌ Kehui Li❌ anniefu
Kehui Li seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

That's a great enhancement! Thanks @kehuili Would you sign the CLA? You might need to get around the GFW to sign it.
And to utilize this python functions-framework, we might need to update https://github.com/OpenFunction/builder as well. Let's discuss this in today's meeting.
Ben

Just signed CLA. Also I picked a commit from GCP for a test fix (or one unit test would fail forever), it seems he/she also need to sign. Do I need to revert this commit and just commit this change myself?

No, just you sign the CLA is all right

@benjaminhuo benjaminhuo requested a review from tpiperatgod May 12, 2022 07:31
Comment on lines 74 to 78
uri = json_dct.get('uri') or ''
component_name = json_dct.get('componentName') or ''
metadata = json_dct.get('metadata')
component_type = json_dct.get('componentType') or ''
operation = json_dct.get('operation') or ''
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to use get("key", "default")?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed.

@tpiperatgod
Copy link
Member

lgtm

@lizzzcai
Copy link
Member

is the quick start example in the readme need to be updated as well?

@benjaminhuo
Copy link
Member

benjaminhuo commented May 13, 2022

Kehui Li seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@kehuili you may need to add your email used to sign this commit as your github primary email and sign the cla again.

https://docs.github.com/cn/account-and-profile/setting-up-and-managing-your-github-user-account/managing-email-preferences/adding-an-email-address-to-your-github-account

OPEN_FUNC_BINDING = "bindings"
OPEN_FUNC_TOPIC = "pubsub"

KNATIVE_RUNTIME_TYPE = "Knative"
Copy link
Member

@benjaminhuo benjaminhuo May 13, 2022

Choose a reason for hiding this comment

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

Is this runtime type used as the runtime field of serving?
If so it should be in lower case knative or async like https://github.com/OpenFunction/samples/blob/main/functions/async/logs-handler-function/logs-handler-function.yaml#L22

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's for runtime in FUNC_CONTEXT, so I thought it should be capitalized w/ first letter like examples here https://github.com/OpenFunction/samples/blob/bb4e664321f0a2e9d509543936abf9a9b776557c/functions-framework/golang/Knative/http/README.md#func_context

@kehuili
Copy link
Collaborator Author

kehuili commented May 13, 2022

is the quick start example in the readme need to be updated as well?

Definitely gonna add some samples later🙆‍♀️

context = _function_registry.get_openfunction_context(None)

# determine if async or knative
if context and context.runtime == ASYNC_RUNTIME_TYPE:
Copy link
Member

Choose a reason for hiding this comment

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

Let's make a case insensitive comparison for xx_RUNTIME_TYPE

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Signed-off-by: Kehui Li <kehui.li@uisee.com>
@kehuili kehuili force-pushed the feature/init_binding_and_async branch from 9444e51 to 830895a Compare May 13, 2022 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants