[feature][functions] Add admin CLI command to get available built-in functions #16822

cbornet · 2022-07-27T15:23:26Z

Motivation

Same as sinks available-sinks and sources available-sources but for Functions.

Modifications

Add HTTP endpoints admin/v3/functions/builtins to broker and function worker.
Add Java admin SDK function getBuiltInFunctions and getBuiltInFunctionsAsync methods.
Add admin CLI command functions available-functions

Verifying this change

Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change added tests and can be verified as follows:

There are no tests for the builtin sinks/source. Shouldn't we add it in another PR ?

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

yes

If yes was chosen, please highlight the changes

The public API: yes
The schema: no
The default values of configurations: no
The wire protocol: no
The rest endpoints: yes
The admin cli options: yes
Anything that affects deployment: no

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)
doc-not-needed
(Please explain why)
doc
(Your PR contains doc changes)
doc-complete
(Docs have been already added)

cbornet · 2022-07-27T15:30:13Z

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/FunctionsImpl.java

    private final AsyncHttpClient asyncHttpClient;

    public FunctionsImpl(WebTarget web, Authentication auth, AsyncHttpClient asyncHttpClient, long readTimeoutMs) {
        super(auth, readTimeoutMs);
        this.functions = web.path("/admin/v3/functions");
+        this.worker = web.path("/admin/v2/worker");


Should available-functions be a command of pulsar-admin workers instead ?
For now, I've put it in pulsar-admin functions for consistency with sinks and sources.

+1 for pulsar-admin functions. However it's a bit weird that the rest endpoint is under "/worker"

This is because we have the list of connectors under /worker.
Now we also have the list of sinks under /sinks and the list of sources under /sources.
So I think we can indeed have the list of functions under /functions

tisonkun

It seems not only "builtin functions", but all available functions?

cbornet · 2022-07-27T16:13:46Z

It's only builtin functions. It's the same as for sinks and sources.
The name available-functions/available-sinks/available-sources are indeed confusing.
We can change to builtin-functions which is more accurate but is not coherent with sinks/sources.

eolivelli

LGTM

nicoloboschi

Good work!

nicoloboschi · 2022-08-17T08:36:39Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Worker.java

+    @ApiResponses(value = {
+            @ApiResponse(code = 200, message = "Get builtin functions successfully.")
+    })
+    @Path("/builtinfunctions")


IMO if we want to follow the same as the "/connectors" above, this endpoint should be "/functions"

I think that "buildinfunctions" is clearer.
"functions" may look like to refer to "all the functions"

not a big deal

nicoloboschi · 2022-08-17T08:37:29Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Worker.java

+            responseContainer = "List"
+    )
+    @ApiResponses(value = {
+            @ApiResponse(code = 200, message = "Get builtin functions successfully.")


what about error responses ?

I added them. PTAL

nicoloboschi · 2022-08-17T08:38:13Z

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/FunctionsImpl.java

    private final AsyncHttpClient asyncHttpClient;

    public FunctionsImpl(WebTarget web, Authentication auth, AsyncHttpClient asyncHttpClient, long readTimeoutMs) {
        super(auth, readTimeoutMs);
        this.functions = web.path("/admin/v3/functions");
+        this.worker = web.path("/admin/v2/worker");


+1 for pulsar-admin functions. However it's a bit weird that the rest endpoint is under "/worker"

nicoloboschi · 2022-08-17T08:39:32Z

pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java

+        void runCmd() throws Exception {
+            getAdmin().functions().getBuiltInFunctions()
+                    .forEach(function -> {
+                        System.out.println(function.getName());


could we create a common method to display this kind of stuff ? (sinks, sources, functions..)

Between source and sink maybe. But between Function and Connectors, we handle distinct classes that are unrelated (FunctionDefinition and ConnectorDefinition) so I don't think we can factorize.

cbornet · 2022-08-17T16:33:46Z

I changed the location of the endpoint from /worker to /functions for more consistency. PTAL.

eolivelli

LGTM

momo-jun · 2022-09-23T07:47:47Z

@cbornet It will help users a lot if you can add the docs for this feature. Do you have any plans on that?

(cherry picked from commit 34eb74d)

cbornet · 2022-11-18T21:36:01Z

Isn't doc for this auto-generated?

tisonkun · 2022-11-19T02:03:49Z

@cbornet Yes. I can read https://pulsar.apache.org/reference/#/next/pulsar-admin/functions?id=available-functions now.

momo-jun · 2022-11-21T02:08:03Z

Yes, the reference doc can be auto-generated now. Besides the reference info, do you have any other additional information to add in the Pulsar doc site to actively promote this feature? If not, the label can be changed to doc-complete.

cbornet · 2022-11-23T10:14:09Z

I opened #18569 to add doc for built-in functions that gives more context.

cbornet · 2022-11-23T15:52:09Z

available-functions and reload have not been picked into 2.11. Will the reference be re-generated with the 2.11 code when it's released ?

tisonkun · 2022-11-23T17:02:28Z

@cbornet I think so. Generally, if we release on time, this won't be a big issue, but 2.11.0 takes more than three months to release. Also, I'm considering versioned docs along with release branches, or cutting new versioned docs on feature freeze (along with cutting the branch), but all of them requires concrete proposals, and I assume to take care of it in months.

For current status, ping @Technoboy- the RM of 2.11.0 FYI. And perhaps we revert the related sections after the docs cut. This is the same as Pulsar SQL's caveat (https://pulsar.apache.org/docs/next/sql-overview#caveat) so you may also add a note for versions as a workaround (we should not have it with a proper process ).

cbornet force-pushed the list-functions branch 2 times, most recently from b6ff1ec to a9055c4 Compare July 27, 2022 15:26

cbornet commented Jul 27, 2022

View reviewed changes

tisonkun reviewed Jul 27, 2022

View reviewed changes

cbornet changed the title ~~Add admin CLI command to get available built-in functions~~ [feature][functions] Add admin CLI command to get available built-in functions Jul 27, 2022

github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Jul 27, 2022

eolivelli approved these changes Aug 17, 2022

View reviewed changes

nicoloboschi requested changes Aug 17, 2022

View reviewed changes

cbornet force-pushed the list-functions branch 2 times, most recently from e4ce5e6 to b851267 Compare August 17, 2022 16:02

cbornet requested review from nicoloboschi and eolivelli August 17, 2022 16:30

cbornet force-pushed the list-functions branch from b851267 to 55a47ea Compare August 18, 2022 18:24

eolivelli approved these changes Aug 24, 2022

View reviewed changes

dlg99 approved these changes Aug 24, 2022

View reviewed changes

nicoloboschi approved these changes Aug 25, 2022

View reviewed changes

cbornet closed this Aug 26, 2022

cbornet reopened this Aug 26, 2022

Add CLI command to get available built-in functions

6333b9c

cbornet force-pushed the list-functions branch from 55a47ea to 6333b9c Compare August 26, 2022 17:02

cbornet closed this Aug 27, 2022

cbornet reopened this Aug 27, 2022

eolivelli merged commit 34eb74d into apache:master Aug 28, 2022

Pomelongan pushed a commit to Pomelongan/pulsar that referenced this pull request Aug 28, 2022

Add CLI command to get available built-in functions (apache#16822)

6d930af

cbornet deleted the list-functions branch August 29, 2022 07:48

Technoboy- assigned cbornet Sep 27, 2022

Technoboy- added this to the 2.12.0 milestone Sep 27, 2022

Technoboy- added type/feature The PR added a new feature or issue requested a new feature area/function labels Sep 27, 2022

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Sep 29, 2022

Add CLI command to get available built-in functions (apache#16822)

35d3f6b

(cherry picked from commit 34eb74d)

momo-jun added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-required Your PR changes impact docs and you will update later. labels Nov 25, 2022

tisonkun mentioned this pull request Nov 25, 2022

Add doc for Pulsar shell clone apache/pulsar-site#303

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature][functions] Add admin CLI command to get available built-in functions #16822

[feature][functions] Add admin CLI command to get available built-in functions #16822

cbornet commented Jul 27, 2022 •

edited by momo-jun

Loading

cbornet Jul 27, 2022 •

edited

Loading

nicoloboschi Aug 17, 2022

cbornet Aug 17, 2022

tisonkun left a comment

cbornet commented Jul 27, 2022

eolivelli left a comment

nicoloboschi left a comment

nicoloboschi Aug 17, 2022 •

edited

Loading

eolivelli Aug 17, 2022

nicoloboschi Aug 17, 2022

cbornet Aug 19, 2022

nicoloboschi Aug 17, 2022

nicoloboschi Aug 17, 2022

cbornet Aug 19, 2022

cbornet commented Aug 17, 2022 •

edited

Loading

eolivelli left a comment

momo-jun commented Sep 23, 2022

cbornet commented Nov 18, 2022

tisonkun commented Nov 19, 2022

momo-jun commented Nov 21, 2022

cbornet commented Nov 23, 2022

cbornet commented Nov 23, 2022

tisonkun commented Nov 23, 2022 •

edited

Loading

[feature][functions] Add admin CLI command to get available built-in functions #16822

[feature][functions] Add admin CLI command to get available built-in functions #16822

Conversation

cbornet commented Jul 27, 2022 • edited by momo-jun Loading

Motivation

Modifications

Verifying this change

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

Documentation

cbornet Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tisonkun left a comment

Choose a reason for hiding this comment

cbornet commented Jul 27, 2022

eolivelli left a comment

Choose a reason for hiding this comment

nicoloboschi left a comment

Choose a reason for hiding this comment

nicoloboschi Aug 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbornet commented Aug 17, 2022 • edited Loading

eolivelli left a comment

Choose a reason for hiding this comment

momo-jun commented Sep 23, 2022

cbornet commented Nov 18, 2022

tisonkun commented Nov 19, 2022

momo-jun commented Nov 21, 2022

cbornet commented Nov 23, 2022

cbornet commented Nov 23, 2022

tisonkun commented Nov 23, 2022 • edited Loading

cbornet commented Jul 27, 2022 •

edited by momo-jun

Loading

cbornet Jul 27, 2022 •

edited

Loading

nicoloboschi Aug 17, 2022 •

edited

Loading

cbornet commented Aug 17, 2022 •

edited

Loading

tisonkun commented Nov 23, 2022 •

edited

Loading