Skip to content

develop ff-python based on ofn v1beta2 #3

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
merged 9 commits into from
Jul 11, 2023

Conversation

tpiperatgod
Copy link
Member

No description provided.

@tpiperatgod tpiperatgod changed the title develop ff-python based on ofn v1beta2 [WIP] develop ff-python based on ofn v1beta2 Jun 21, 2023
@benjaminhuo
Copy link
Member

That's great, looking forward to this PR! @wanjunlei @wrongerror

Signed-off-by: laminar <[email protected]>
Signed-off-by: laminar <[email protected]>
Signed-off-by: laminar <[email protected]>
@tpiperatgod
Copy link
Member Author

I think this PR is ready for review, as it currently supports Dapr and HTTP triggers. (can be demonstrated in the sig meeting)

The observability feature and the hooks feature can be worked on later.

What are your comments?

@tpiperatgod tpiperatgod changed the title [WIP] develop ff-python based on ofn v1beta2 develop ff-python based on ofn v1beta2 Jul 6, 2023
@benjaminhuo
Copy link
Member

@tpiperatgod That's great. @wanjunlei @wrongerror @webup @lizzzcai would you help to review?

@benjaminhuo benjaminhuo requested review from lizzzcai and kehuili and removed request for kehuili July 7, 2023 01:58
@tpiperatgod tpiperatgod linked an issue Jul 7, 2023 that may be closed by this pull request
return self.__http_request

@exception_handler
def send(self, output_name, data):
Copy link
Member

@benjaminhuo benjaminhuo Jul 8, 2023

Choose a reason for hiding this comment

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

Now we do not encourage users to call send now. Instead, we suggest invoking dapr method directly using daprClient like this https://github.com/OpenFunction/java-samples/blob/main/src/main/java/dev/openfunction/samples/StateStore.java#L72

invokeBindings for the binding component

Copy link
Member Author

Choose a reason for hiding this comment

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

I see. Should we remove send() or keep it for now?

Copy link
Member

@benjaminhuo benjaminhuo Jul 10, 2023

Choose a reason for hiding this comment

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

We can keep it for now, but can user get daprClient from function's context now?

Copy link
Member Author

Choose a reason for hiding this comment

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

UserContext has the self.dapr_client

Copy link
Member

Choose a reason for hiding this comment

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

UserContext has the self.dapr_client

Then we need samples to call dapr client directly

@benjaminhuo
Copy link
Member

I'm okay with this PR, and it's great to have this significant improvement to the Python framework.
Just some minor issues remain.

@lizzzcai
Copy link
Member

lizzzcai commented Jul 9, 2023

any changes from the user side? maybe can update the docs if any.

@tpiperatgod
Copy link
Member Author

any changes from the user side? maybe can update the docs if any.

I'll update the docs and examples later

@@ -0,0 +1,217 @@
# Copyright 2020 Google LLC
Copy link
Member

Choose a reason for hiding this comment

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

this should be updated as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I kept some of gcp's license header because almost all of that code comes from gcp's ff-python, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

in this case, I think can keep it. maybe @benjaminhuo can comment.

Copy link
Member

Choose a reason for hiding this comment

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

Keep it if we haven't modify it

Signed-off-by: laminar <[email protected]>
@benjaminhuo benjaminhuo merged commit 4c57790 into OpenFunction:master Jul 11, 2023
@tpiperatgod tpiperatgod deleted the dev branch July 11, 2023 02:46
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.

Support ofn v1beta2
3 participants