[feature/agent_framework] Changing resources created format

[amitgalitz]

Description

Added updates to resources_created section for register_model_group, register model, create index, create ingest pipeline and create connector steps.

Still didn't add this for register agent since there is more changes that will be merged for that soon.

I also added retryOnConflict here based on same number as ml-commons has, since adding this I haven't seen the version conflict issues but I will continue to see if there are better concurrency controls we can have, added a TODO for this.

Changed the format of creating a resource to:

 {
                "_index": ".plugins-workflow-state",
                "_id": "RvEFF4wBKbeLfRazqAdh",
                "_score": 1.0,
                "_source": {
                    "workflow_id": "RvEFF4wBKbeLfRazqAdh",
                    "state": "PROVISIONING",
                    "provisioning_progress": "IN_PROGRESS",
                    "provision_start_time": 1701193321252,
                    "resources_created": [
                        {
                            "workflow_step_name": "create_connector",
                            "workflow_step_id": "workflow_step_1-amit-test",
                            "connector_id": "TfEGF4wBKbeLfRazCwcp"
                        },
                        {
                            "workflow_step_name": "register_remote_model",
                            "workflow_step_id": "workflow_step_2",
                            "model_id": "T_EGF4wBKbeLfRazCwdx"
                        }
                    ],
                    "provision_end_time": 1701193305145
                }
            }

Issues Resolved

resolves #173
resolves #136

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[joshpalis]

Initial pass, looks great, but just had a few thoughts

[dbwiddis]

Initial pass

[dbwiddis]

Are we sure we will only ever create a single resource in a given step? Why not return a list here?

[amitgalitz]

Right now I haven't seen a step that returns multiple resources. It makes it a little more complicated if we have to do a mapping of step 1 creates two fields "connector_id" and "model_id" for example and then we map each ID to the correct one when updating. If you think its likely we will have a step like this I can change the logic

[amitgalitz]

I could add this with my PR for converting the workflow-step.json to an enum if that is okay.

[amitgalitz]

tried this out and there are some things to still decide on and I can expand in an issue. Basically making sure we map the right resource key to the right resource value could be a little tricky. For example if we have a list of resourcesCreatedTypes like (“connector_id”, “model_id”, “detector_id”). We need to create a map with each one of those as the key and to know which resource to get but that is arbitrary on how the resource looks like in the response.

 List<String> resourceNames = WorkflowResources.getResourcesByWorkflowStep(getName());
 Map<String, Object> map = resourceNames.stream()
            .collect(Collectors.toMap(
                    key -> key,                       // Key Mapper: Use the list item as the key.
                    value -> {//getting correct corresponding value might not be straightforward cause we would}
                        // a mapping of each possible resource to the correct method for example"
                        // "connector_id" -> mlCreateConnectorResponse.getConnectorId()
                        // "detector" -> however that client defines it as and etc. 
            ));

[owaiskazi19]

I agree with @dbwiddis here. We should return a list rather a single resource. Follow up PR should be fine.

[owaiskazi19]

@amitgalitz why not create a global map here like we had List<WorkflowData> in the execute method, name it as Map<String, ResourcesCreated> and store the resources of a step in it as a new entry to the list.

This way with status API, we need to just iterate on the map which would have key as workflow_step_name and values as the resources created. It will be easy with the new Util method @dbwiddis has here #234

[dbwiddis]

Why don't we keep the resource type together with its ID? In order to know the meaning of the resource ID here I need to know its step. This way forces me to look it up from the enum (assuming a 1:1 mapping which I'm not sure of).

[amitgalitz]

What do you mean by resource type here? the field of connector_id will be the key here for resourceId. Since we wanted to have a dynamic key it means it could be anything so I label it as resourceId here

[joshpalis]

Great work @amitgalitz ! Thanks for addressing the changes

[owaiskazi19]

Overall looks good with a general comment of changing the exception to FlowFrameworkException.