[FEATURE] Add capability to add optional api path params

[amitgalitz]

Is your feature request related to a problem?

Some APIs have optional additional params in there api path. For example: POST /_plugins/_ml/models/_register API has option to add ?deploy=true or GET _plugins/_anomaly_detection/detectors/<detectorId>/_profile?_all=true API has the ?_all=true as an option.

What solution would you like?

We should have a standard way of taking in this optional params as part of our template, whether it be designated in a workflow step like:

{
          "id": "workflow_step_2",
          "type": "register_model",
          "previous_node_inputs": {
            "workflow_step_1": "connector_id"
          },
          "user_inputs": {
            "name": "openAI-gpt-3.5-turbo",
            "function_name": "remote",
            "description": "test model",
            "optional_api_params" : {
                "deploy": true
            }
        }

or some other mechanism to signify both that this is an optional parameter and that it is a part of the API path not the request payload.

We can also add this information on the workflow-step.json source of truth in the backend to register these optional params for the steps that require them.

[dbwiddis]

I've proposed a way to handle optional params in #234, which would not treat them any differently than any other params, they're just internally marked as required in the step. So you'd just have "deploy": "true" on the same level as description, etc.

Set<String> requiredKeys = Set.of(NAME_FIELD, FUNCTION_NAME, CONNECTOR_ID);
Set<String> optionalKeys = Set.of(MODEL_GROUP_ID, DESCRIPTION_FIELD);

Although I think @owaiskazi19 may have a competing implementation PR up later today.

We can also add this information on the workflow-step.json source of truth

Yep (or an equivalent hardcoded version that we export that from).

optional params for the steps that require them.

If they're optional they're not required, right? :-)

[dbwiddis]

@amitgalitz is this still something that needs to be done or does the existing required/optional keys satisfy it?

[amitgalitz]

@amitgalitz is this still something that needs to be done or does the existing required/optional keys satisfy it?

I might be wrong but I think the optional keys satisfy almost all cases, for example if we want to add this API:

GET _plugins/_anomaly_detection/detectors/<detectorId>?job=true

we would be able to add job: true as another key value pair in the user_inputs.

However I only really thought of this case now but I was wondering if we ever wanted to add an API like this:
_cat/nodes?h=name,ip,role would we need to make some advancements or would giving h: [name, ip, role] work?

We currently don't have any API onboarded that uses this as its mostly for GET apis but we can either have a separate issue for that or utilize this one if we currently wont support this out of the block. Also fair to only really label this when we encounter such an API and not over-engineer

[dbwiddis]

However I only really thought of this case now but I was wondering if we ever wanted to add an API like this: _cat/nodes?h=name,ip,role would we need to make some advancements or would giving h: [name, ip, role] work?

We'd have to manually parse the param (String.split(",")). I actually envision this as something to do with the validation checks. Currently we just check for 'all' but we could separate out plugin validation for example.

I think the biggest concern here is that if you look at the REST API docs corresponding to some of our steps there are a lot more params included, we only use a subset of them in many cases.

[dbwiddis]

@amitgalitz revisiting this in response to this comment

can we parameterize the workflow so that it could be invoked with different parameters (similar to the saved search API) ?

In the observability example we may want to perform the same workflow steps on multiple different workflows, so you'd have a setup where you'd create a workflow and when you provision it, you'd pass a parameter.

So for example you'd call

POST /_plugins/_flow_framework/workflow/bhPBgI0BQfVbnJKtHwGU/_provision?index_name=foo

Somewhere in the template you'd reference "index_name" somehow, likely as a substitution. Two main approaches I see.

1. We could use a fixed substitution prefix, e.g., ${{path_param.index_name}}.
2. We could have a section in the template with required or optional path params, where it would then behave similarly to previous_node_inputs to use that value for the key, e.g.,

{
  "id": "reindex_old_otel_spans_index_to_new",
  "type": "rest_api",
  "previous_node_inputs": {
    "http_client": "client"
  },
  "required_path_params" : ["index_name"]
  "method": "POST",
  "endpoint": "_reindex",
  "body": {
    "source": {
      "index": "otel-v1-apm-span-*"
    },
    "dest": {
      "index": "${{index_name}}"
    },
    "script": {
      "source": "ctx._source['@timestamp'] = ctx._source.startTime;"
    }
  }
}

The second option would allow us to validate the params in the call before executing the workflow (by including "required_path_params"). We already include the ${{ ... }} substitution in our input parsing, the only difference here is that we'd pass along the params() from the path and use those to populate the map to ensure that key exists for the substitution.

@amitgalitz, @YANG-DB, WDYT?

[dbwiddis]

@amitgalitz I see you proposed something similar to my above approach in #496 but with the key/value in the body rather than path.

This could work but adds the complexity of parsing the body into a key-value map.

[owaiskazi19]

@dbwiddis Adding optional parameters to the path can overload the path.

I liked what @amitgalitz has proposed in the issue with the key/value pair

"optional_api_params" : {
                "deploy": true
 }

This way we can read any number of optional params and making the path looks much simpler.

[dbwiddis]

Adding optional parameters to the path can overload the path.

Why is this a problem?

I liked what @amitgalitz has proposed in the issue with the key/value pair

This doesn't address the use case of a user creating a template with an arbitrary number of params we do not know in advance.

This way we can read any number of optional params and making the path looks much simpler.

I can make path or body optional (with body style described in #496) but I don't want to prevent people from using whichever one they want.

[owaiskazi19]

Why is this a problem?

If we have a template with multiple steps, the question arises: How do we assign parameters for each workflow step? With the proposed implementation, is there a mechanism to designate an associated workflow step before the parameter?

For example:

       {
          "id": "workflow_step_1",
          "type": "register_model",
          "previous_node_inputs": {
            "workflow_step_1": "connector_id"
          },
          "user_inputs": {
            "name": "${{ name }}",
            "function_name": "remote",
            "description": "test model",
        },
        {
          {
          "id": "workflow_step_2",
          "type": "register_model",
          "previous_node_inputs": {
            "workflow_step_1": "connector_id"
          },
          "user_inputs": {
            "name": "${{ name }}",
            "function_name": "remote",
            "description": "test model",
        },
        }

If we pass

POST /_plugins/_flow_framework/workflow/bhPBgI0BQfVbnJKtHwGU/_provision?name=foo

wouldn't name be replaced for both the workflow steps here?

I can make path or body optional (with body style described in #496) but I don't want to prevent people from using whichever one they want.

In #496, the focus is on making our sample templates dynamic. Some fields can be made dynamic, necessitating user input. For the remaining fields, if the user provides input, we can update them; otherwise, we can use the default values. This way we can reduce the size of the payload for our create workflow API.

It's fine we don't want to prevent people from using whichever they want, but that brings up the same question I stated above.

[dbwiddis]

wouldn't name be replaced for both the workflow steps here?

Yes, if that's what's wanted. If not, then have the names be different, e.g.,

       {
          "id": "workflow_step_1",
          "type": "register_model",
          "previous_node_inputs": {
            "workflow_step_1": "connector_id"
          },
          "user_inputs": {
            "name": "${{ step_1.name }}",
            "function_name": "remote",
            "description": "test model",
        },
        {
          {
          "id": "workflow_step_2",
          "type": "register_model",
          "previous_node_inputs": {
            "workflow_step_1": "connector_id"
          },
          "user_inputs": {
            "name": "${{ step_2.name }}",
            "function_name": "remote",
            "description": "test model",
        },

and call with

POST /_plugins/_flow_framework/workflow/bhPBgI0BQfVbnJKtHwGU/_provision?step_1.name=foo&step_2.name=bar

or

POST /_plugins/_flow_framework/workflow/bhPBgI0BQfVbnJKtHwGU/_provision
{
  "step_1.name": "foo",
  "step_2.name": "bar"
}

or

POST /_plugins/_flow_framework/workflow/bhPBgI0BQfVbnJKtHwGU/_provision?step_1.name=foo
{
  "step_2.name": "bar"
}

[dbwiddis]

@amitgalitz I think between the use case templates with defaults and the substitutions, this has been completed, do you agree?

[amitgalitz]

Yes I believe so