You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/02/23 08:58:03 UTC
[GitHub] [pulsar] yaalsn opened a new pull request #14428: [functions] Add executor for CmdFunctions
yaalsn opened a new pull request #14428:
URL: https://github.com/apache/pulsar/pull/14428
Fixes #14388
### Motivation
In pulsar function, when the runtime is `Python` or `Java` , commands will start with `python` or `java`
<img width="795" alt="image" src="https://user-images.githubusercontent.com/10069311/155286587-37d97742-b28a-4590-bb58-591166d11334.png">
This would be right in general, but sometimes is not such as `/home/pulsar/jdk17/bin/java`, `python3` and `conda` `pyenv` python virutalenv. We need to add a executor to let user to choose.
### Modifications
- Add `--executor` parameter to `CmdFunctions`.
- Add `executor` to `functions.proto`
- Replace `java` and `python` with executor
### Verifying this change
- [x] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
- *Extend unit tests for args which contains executor to be parsed*
- *Added integration tests for function run with executor*
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (yes / **no**)
- The public API: (yes / **no**)
- The schema: (yes / no / don't know)
- The default values of configurations: (yes / no)
- The wire protocol: (yes / **no**)
- The rest endpoints: (yes / **no**)
- **The admin cli options**: (**yes** / no)
- Anything that affects deployment: (yes / **no** / don't know)
### Documentation
Need to update docs?
- [x] `doc`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yaalsn closed pull request #14428: [functions] Add executor for CmdFunctions
Posted by GitBox <gi...@apache.org>.
yaalsn closed pull request #14428:
URL: https://github.com/apache/pulsar/pull/14428
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yaalsn commented on pull request #14428: [functions] Add executor for CmdFunctions
Posted by GitBox <gi...@apache.org>.
yaalsn commented on pull request #14428:
URL: https://github.com/apache/pulsar/pull/14428#issuecomment-1048567545
@codelipenghui @Anonymitaet PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] yaalsn commented on pull request #14428: [functions] Add executor for CmdFunctions
Posted by GitBox <gi...@apache.org>.
yaalsn commented on pull request #14428:
URL: https://github.com/apache/pulsar/pull/14428#issuecomment-1049696896
> @yaalsn normally after the PR gets approved by other coders (technical perspective), then tech writers will review it from the technical writing perspective. So could you please ping me to review it after gets technical approval? Thanks
ok, got it 👌
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #14428: [functions] Add executor for CmdFunctions
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #14428:
URL: https://github.com/apache/pulsar/pull/14428#issuecomment-1049358907
Thanks for the contribution, but I think this PR needs more discussion before merge it, as from the Pulsar Functions' user aspect, they dont always know the enviroment settings from the server side, and the user may submit wrong executor path. So it would be better if the executor path be set from the server side (i.e. new configs in Functions Worker configs) but not from the user's side.
@nlu90 @jerrypeng @wolfstudy PTAL, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #14428: [functions] Add executor for CmdFunctions
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14428:
URL: https://github.com/apache/pulsar/pull/14428#issuecomment-1079814925
The pr had no activity for 30 days, mark with Stale label.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #14428: [functions] Add executor for CmdFunctions
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #14428:
URL: https://github.com/apache/pulsar/pull/14428#issuecomment-1049694150
@yaalsn normally after the PR gets approved by other coders (technical perspective), then tech writers will review it from the technical writing perspective. So could you please ping me to review it after gets technical approval? Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet edited a comment on pull request #14428: [functions] Add executor for CmdFunctions
Posted by GitBox <gi...@apache.org>.
freeznet edited a comment on pull request #14428:
URL: https://github.com/apache/pulsar/pull/14428#issuecomment-1049358907
Thanks for the contribution, but I think this PR needs more discussion before merge it, as from the Pulsar Functions' user aspect, they dont always know the enviroment settings from the server side, and the user may submit wrong executor path or vulnerable path. So it would be better if the executor path be set from the server side (i.e. new configs in Functions Worker configs) but not from the user's side.
@nlu90 @jerrypeng @wolfstudy PTAL, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org