Skip to content
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

[improve][admin] Allow creating builtin functions in pulsar-admin CLI #15671

Merged
merged 1 commit into from
May 25, 2022

Conversation

cbornet
Copy link
Contributor

@cbornet cbornet commented May 19, 2022

Motivation

Currently creating a built-in function with argument --jar builtin://my-built-in-function is rejected with error The specified jar file does not exist.
This change allows to do it.

Modifications

Check if the jar starts with builtin://

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

Run test CmdFunctionsTest::testCreateFunctionWithBuiltinNar

Does this pull request potentially affect one of the following parts:

No

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

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • no-need-doc
    trivial (or would need a full doc on builtin functions which is inexistant atm)

  • doc
    (Your PR contains doc changes)

  • doc-added
    (Docs have been already added)

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 19, 2022
@cbornet cbornet force-pushed the builtin-functions-cli branch from 81052b0 to a9597a9 Compare May 19, 2022 12:57
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

I had not understood well the feature. As discussed offline now the feature is very clear to me and works well.

Supporting to not set the classname can be done as a follow up step

@nicoloboschi
Copy link
Contributor

/pulsarbot rerun-failure-checks

Copy link
Contributor

@dlg99 dlg99 left a comment

Choose a reason for hiding this comment

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

LGTM

@cbornet cbornet changed the title Allow creating builtin functions in pulsar-admin CLI [improve][function] Allow creating builtin functions in pulsar-admin CLI May 21, 2022
@cbornet cbornet changed the title [improve][function] Allow creating builtin functions in pulsar-admin CLI [improve][admin] Allow creating builtin functions in pulsar-admin CLI May 21, 2022
@cbornet cbornet force-pushed the builtin-functions-cli branch from a9597a9 to f86f277 Compare May 21, 2022 14:26
@nicoloboschi nicoloboschi merged commit 6560bb0 into apache:master May 25, 2022
@nicoloboschi nicoloboschi added this to the 2.11.0 milestone May 25, 2022
@cbornet cbornet deleted the builtin-functions-cli branch May 25, 2022 10:41
cbornet added a commit to cbornet/pulsar that referenced this pull request Jun 17, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jun 17, 2022
* Parse function userConfig to Map<String, Object> (apache#15669)

Currently only {userConfig: {some-key: some-value}} is allowed which is limiting
With this change any JSON compatible object is allowed. Eg: {userConfig: {some-key: [value1, value2]}}

* Allow creating builtin functions in pulsar-admin CLI (apache#15671)

* [fix][admin] Fix check on create function class name (apache#15700)

The check was done for PYTHON or JAVA runtime but the runtime is not inferred in the admin CLI.
The fix is to do the check if a jar or py is provided instead

* [improve][function] Get function class name from the NAR when using built-in functions (apache#15693)

* Get function class name from the NAR manifest when using built-in functions

* Exclude pulsar-io.yaml from examples JAR

* Build a NAR separately from the JAR for the function examples

* Improve admin cli tes for built-in functions
cbornet added a commit to datastax/pulsar that referenced this pull request Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admin area/function doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants