Add stats, status, querystate, putstate, trigger cmds for Pulsar Functions #34

wolfstudy · 2019-09-05T08:50:01Z

Master Issue: #2

Add stats, status, querystate, putstate, trigger cmds for Pulsar Functions

…tions Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

sijie · 2019-09-05T18:37:20Z

please make sure it pass CI

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

wolfstudy · 2019-09-06T03:48:46Z

@sijie PTAL again

sijie · 2019-09-06T06:00:14Z

pkg/ctl/functions/putstate.go

+
+func putstateFunctionsCmd(vc *cmdutils.VerbCmd) {
+	desc := pulsar.LongDescription{}
+	desc.CommandUsedFor = "Put the state associated with a Pulsar Function."


Suggested change

desc.CommandUsedFor = "Put the state associated with a Pulsar Function."

desc.CommandUsedFor = "Put a key/value pair to the state associated with a Pulsar Function."

The FunctionState as follows:

type FunctionState struct { Key string `json:"key"` StringValue string `json:"stringValue"` ByteValue []byte `json:"byteValue"` NumValue int64 `json:"numberValue"` Version int64 `json:"version"` }

It isn't a key/value pair.

Fundamentally, the state of a function is a key/value map. The value is converted and stored as bytes under the hood. You can interpret the value as string, byte[] or number. The version is assigned when you put the key/value pair to the state.

The Pulsar state related restful api is not done well. For example, we shouldn't pass any version number when putting a key/value pair to the state; only one of the value fields (StringValue, ByteValue and NumValue) should be set.

The pulsar-admin state command is also bad because you shouldn't let user construct a json string in the command. That's a very bad user experience.

The good putstate commands should be:

// put a key/string-value pair to state pulsarctl functions putstate key string-value // put a key/string-value pair to the state ('hello word' is treated as the value) pulsarctl functions putstate key - hello world // put a key/number-value pair to the state pulsarctl functions putstate key 12345 // put a key/bytes-value pair to the state pulsarctl functions putstate key < /path/to/value/file

The pulsarctl should interpret the value and set it to the right field in FunctionState struct and post it to the broker.

sijie · 2019-09-06T06:00:46Z

pkg/ctl/functions/putstate.go

+func putstateFunctionsCmd(vc *cmdutils.VerbCmd) {
+	desc := pulsar.LongDescription{}
+	desc.CommandUsedFor = "Put the state associated with a Pulsar Function."
+	desc.CommandPermission = "This command requires super-user permissions."


why does this command requires super-user permission?

sijie · 2019-09-06T06:01:13Z

pkg/ctl/functions/putstate.go

+
+	var examples []pulsar.Example
+	putstate := pulsar.Example{
+		Desc: "Put the state associated with a Pulsar Function",


Put a key/value pair to the state associated with a Pulsar Function

sijie · 2019-09-06T06:01:48Z

pkg/ctl/functions/putstate.go

+	examples = append(examples, putstate)
+
+	putstateWithFQFN := pulsar.Example{
+		Desc: "Put the state associated with a Pulsar Function with FQFN",


please update the description. see my comment above.

pkg/ctl/functions/putstate.go

sijie · 2019-09-06T06:09:20Z

pkg/ctl/functions/stats.go

+func statsFunctionsCmd(vc *cmdutils.VerbCmd) {
+	desc := pulsar.LongDescription{}
+	desc.CommandUsedFor = "Get the current stats of a Pulsar Function."
+	desc.CommandPermission = "This command requires super-user permissions."


sijie · 2019-09-06T06:10:08Z

pkg/ctl/functions/stats_test.go

+	assert.Equal(t, out.String(), "Created test-functions-stats successfully")
+
+	// wait the function create successfully
+	time.Sleep(time.Second * 3)


avoid sleeping. use a retry-loop to check the stats.

sijie · 2019-09-06T06:10:27Z

pkg/ctl/functions/status_test.go

+	assert.Equal(t, out.String(), "Created test-functions-status successfully")
+
+	// wait the function create successfully
+	time.Sleep(time.Second * 30)


avoid sleeping

sijie · 2019-09-06T06:11:30Z

pkg/ctl/functions/trigger.go

+	}
+	admin := cmdutils.NewPulsarClientWithApiVersion(pulsar.V3)
+
+	if funcData.TriggerValue == "" && funcData.TriggerFile == "" {


what about if a user provides both arguments?

If a user provides both arguments, priority use of triggerFile, code logic as follows:

if (uploadedInputStream != null) { targetArray = new byte[uploadedInputStream.available()]; uploadedInputStream.read(targetArray); } else { targetArray = input.getBytes(); }

uploadedInputStream => triggerFile

input => triggerValue

@wolfstudy that's confusing. please fail the command if both options are provided.

sijie · 2019-09-06T06:11:47Z

pkg/ctl/functions/trigger_test.go

+	assert.Equal(t, out.String(), "Created test-functions-trigger successfully")
+
+	// wait the function create successfully
+	time.Sleep(time.Second * 50)


avoid sleeping

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

sijie · 2019-09-07T05:50:37Z

pkg/ctl/functions/putstate.go

+func putstateFunctionsCmd(vc *cmdutils.VerbCmd) {
+	desc := pulsar.LongDescription{}
+	desc.CommandUsedFor = "Put a key/value pair to the state associated with a Pulsar Function."
+	desc.CommandPermission = "This command requires user permissions."


Let's standardize the naming for permission:

super-user permission

namespace admin permissions

namespace produce permissions

namespace consume permissions

namespace function permissions

So this command requires namespace function permissions.

sijie · 2019-09-07T05:54:45Z

pkg/ctl/functions/putstate.go

+			"",
+			"The name of a Pulsar Function")
+
+		flagSet.StringVarP(


@wolfstudy so what is the take here? are we still using --state with a json string?

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

sijie · 2019-09-07T20:06:32Z

pkg/ctl/functions/putstate.go

+	examples = append(examples, putstate)
+
+	putstateWithByte := pulsar.Example{
+		Desc: "Put a key/<byte value> pair to the state associated with a Pulsar Function",


I don't think we should treat this case as byte value case

sijie · 2019-09-07T20:06:59Z

pkg/ctl/functions/putstate.go

+		Out:  "[✖]  code: 404 reason: Function <your function name> doesn't exist",
+	}
+
+	failOutWithWrongJson := pulsar.Output{


If we remove --state, I don't think we will hit this error.

sijie · 2019-09-07T20:16:47Z

pkg/ctl/functions/putstate.go

+	state.Key = vc.NameArgs[0]
+	value := vc.NameArgs[1]
+
+	if value == "-" {


please take a look at kubectl.

# List contents of /usr from the first container of pod 123456-7890 and sort by modification time. # If the command you want to execute in the pod has any flags in common (e.g. -i), # you must use two dashes (--) to separate your command's flags/arguments. # Also note, do not surround your command and its flags/arguments with quotes # unless that is how you would execute it normally (i.e., do ls -t /usr, not "ls -t /usr"). kubectl exec 123456-7890 -i -t -- ls -t /usr

As I commented before, I was suggesting all the named args after - (we can use -- similar as kubectl as well) are the string value.

I will explain that using following examples as below:

// the key is `key-1`, the string-value is `value-1` pulsarctl functions putstate key-1 value-1 // throw exception since we are expecting one string-value pulsarctl functions putstate key-1 value-1 value-2 // not sure how the framework work here. but ideally `value-1 value` should be treated as one name arg. // if so, the string-value should be `value-1 value2`. pulsarctl functions putstate key-1 "value-1 value2" // in this example, all the name args after `--` should be treated as one string-value "hello world pulsar ctl!" pulsarctl functions putstate key-1 -- hello world pulsar ctl!

for bytes-value, I was suggesting using > as separator. but I forgot > is a special character in bash. we can use a different separator. maybe ---?

We use = as the separator between key/byteValue pair?

> or < means input or output redirection
-- This marks end of parameter (option) list.
--- bad flag syntax: ---

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

wolfstudy · 2019-09-09T02:55:01Z

@sijie PTAL again, thanks

…tions (streamnative/pulsarctl#34) Master Issue: streamnative/pulsarctl#2 Add `stats`, `status`, `querystate`, `putstate`, `trigger` cmds for Pulsar Functions

wolfstudy added 2 commits September 5, 2019 16:32

Add stats, status, querystate, putstate, trigger cmds for Pulsar Func…

a1dad2d

…tions Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

merge master code

1ad77e5

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

wolfstudy requested review from sijie and zymap September 5, 2019 08:50

wolfstudy self-assigned this Sep 5, 2019

wolfstudy added the area/pulsar/functions label Sep 5, 2019

wolfstudy added 3 commits September 5, 2019 20:43

debug ci error

63586d4

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

debug ci error

368311c

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

fix ci error

16c613a

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

wolfstudy added 4 commits September 6, 2019 10:01

fix ci error

ffc24b0

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

fix ci error

0cc45af

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

fix ci error

3b0a0cb

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

fix ci error

228ea17

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

zymap approved these changes Sep 6, 2019

View reviewed changes

sijie requested changes Sep 6, 2019

View reviewed changes

wolfstudy added 3 commits September 6, 2019 17:18

fix comments

6c231fd

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

fix comments

f2ab31e

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

fix comments

bd416cc

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

sijie reviewed Sep 7, 2019

View reviewed changes

fix comments

7750959

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

sijie requested changes Sep 7, 2019

View reviewed changes

fix comments

665dc63

Signed-off-by: xiaolong.ran <ranxiaolong716@gmail.com>

sijie approved these changes Sep 9, 2019

View reviewed changes

sijie added this to the 0.0.1 milestone Sep 9, 2019

sijie merged commit bb9c79d into master Sep 9, 2019

sijie deleted the xiaolong/debug-pulsar-functions branch September 9, 2019 06:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add stats, status, querystate, putstate, trigger cmds for Pulsar Functions #34

Add stats, status, querystate, putstate, trigger cmds for Pulsar Functions #34

wolfstudy commented Sep 5, 2019 •

edited by sijie

Loading

sijie commented Sep 5, 2019

wolfstudy commented Sep 6, 2019

sijie Sep 6, 2019

wolfstudy Sep 6, 2019

sijie Sep 6, 2019

sijie Sep 6, 2019

wolfstudy Sep 6, 2019

sijie Sep 6, 2019

sijie Sep 6, 2019

sijie Sep 6, 2019

sijie Sep 6, 2019

sijie Sep 6, 2019

sijie Sep 6, 2019

wolfstudy Sep 6, 2019 •

edited

Loading

sijie Sep 6, 2019

wolfstudy Sep 6, 2019

sijie Sep 6, 2019

sijie Sep 7, 2019

sijie Sep 7, 2019

sijie Sep 7, 2019

sijie Sep 7, 2019

sijie Sep 7, 2019

wolfstudy Sep 9, 2019 •

edited

Loading

wolfstudy commented Sep 9, 2019

	desc.CommandUsedFor = "Put the state associated with a Pulsar Function."
	desc.CommandUsedFor = "Put a key/value pair to the state associated with a Pulsar Function."

Add stats, status, querystate, putstate, trigger cmds for Pulsar Functions #34

Add stats, status, querystate, putstate, trigger cmds for Pulsar Functions #34

Conversation

wolfstudy commented Sep 5, 2019 • edited by sijie Loading

sijie commented Sep 5, 2019

wolfstudy commented Sep 6, 2019

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

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

wolfstudy Sep 6, 2019 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wolfstudy Sep 9, 2019 • edited Loading

Choose a reason for hiding this comment

wolfstudy commented Sep 9, 2019

wolfstudy commented Sep 5, 2019 •

edited by sijie

Loading

wolfstudy Sep 6, 2019 •

edited

Loading

wolfstudy Sep 9, 2019 •

edited

Loading