-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Issue #8268][Pulsar Function] k8s runtime with go functions support #8352
[Issue #8268][Pulsar Function] k8s runtime with go functions support #8352
Conversation
15b1220
to
cad7e66
Compare
@freeznet Can you add an integration test for this? |
sure, wip |
f873c8e
to
e6599bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM +1
/pulsarbot run-failure-checks |
e6599bf
to
e87e125
Compare
/pulsarbot run-failure-checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change overall looks good. But I think we need to add an integration test to ensure this work.
@@ -52,7 +52,7 @@ public String getSecretsProviderClassName(Function.FunctionDetails functionDetai | |||
case PYTHON: | |||
return "secretsprovider.EnvironmentBasedSecretsProvider"; | |||
case GO: | |||
throw new UnsupportedOperationException(); | |||
return ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to create an issue and add a comment here to link to the issue. Otherwise, we will eventually forget to add secrets provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have created a new issue #8425
@sijie thanks for your review. currently the pulsar repo do not have any integration test implemented with k8s runtime, maybe we should create a new issue to address the lack of integration test for k8s runtime. once the integration tests with k8s runtime implemented, then we can manage the integration tests with k8s runtime & golang functions. |
e87e125
to
34ded0e
Compare
/pulsarbot run-failure-checks |
1 similar comment
/pulsarbot run-failure-checks |
34ded0e
to
bbbf01e
Compare
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
3 similar comments
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
|
…pport (apache#8352) Fixes apache#8268 ### Motivation currently, go function cannot work with k8s runtime other than java and python, this PR is intended to add go function support with k8s runtime. ### Modifications removed `UnsupportedOperationException` with GO function, fixed go function executable permissions, and fix arguments passed to go function with correct format. this PR is making k8s runtime with go function workable, but may not cover all k8s scenarios, so comments are welcome, also need some help with tests, havnt add any tests yet.
depends by #8780 |
…8352) Fixes #8268 ### Motivation currently, go function cannot work with k8s runtime other than java and python, this PR is intended to add go function support with k8s runtime. ### Modifications removed `UnsupportedOperationException` with GO function, fixed go function executable permissions, and fix arguments passed to go function with correct format. this PR is making k8s runtime with go function workable, but may not cover all k8s scenarios, so comments are welcome, also need some help with tests, havnt add any tests yet. (cherry picked from commit bd475c2)
Fixes #8268
Motivation
currently, go function cannot work with k8s runtime other than java and python, this PR is intended to add go function support with k8s runtime.
Modifications
removed
UnsupportedOperationException
with GO function, fixed go function executable permissions, and fix arguments passed to go function with correct format.this PR is making k8s runtime with go function workable, but may not cover all k8s scenarios, so comments are welcome, also need some help with tests, havnt add any tests yet.
Verifying this change
This change added tests and can be verified as follows:
tests WIP