Added integrations tests for checking workflow creation and update sc… #1
Conversation
…enario Signed-off-by: Stevan Buzejic <[email protected]>
Signed-off-by: Stevan Buzejic <[email protected]>
| @@ -1,4 +1,3 @@ | |||
| package org.opensearch.alerting.model.workflow | |||
|
|
|||
| data class WorkflowRunResult { | |||
There was a problem hiding this comment.
we should return list of monitor run results within this class with info from each delegate monitor run.
There was a problem hiding this comment.
we should return list of monitor run results within this class with info from each delegate monitor run.
Yeah - I agree. That's in another PR that contains execution of the workflow changes. Check it out here
| import org.opensearch.rest.RestStatus | ||
| import org.opensearch.tasks.Task | ||
| import org.opensearch.transport.TransportService | ||
|
|
There was a problem hiding this comment.
let's add java docs for classes and comments for methods having complicated logic
There was a problem hiding this comment.
I agree. This class is pretty much obvious and the methods inside it are pretty much descriptive. So probably for indexing the workflow
|
|
||
| val user = readUserFromThreadContext(client) | ||
| val deleteRequest = DeleteRequest(ScheduledJob.SCHEDULED_JOBS_INDEX, transformedRequest.workflowId) | ||
| .setRefreshPolicy(transformedRequest.refreshPolicy) |
There was a problem hiding this comment.
plz remove refresh policy
we will always choose to refresh immediately.
|
|
||
| if (canDelete) { | ||
| val deleteResponse = deleteWorkflow(workflow) | ||
| // TODO - uncomment once the workflow metadata is added |
There was a problem hiding this comment.
Well no - because the workflow metadata are not added still
| } | ||
|
|
||
| private suspend fun deleteWorkflow(workflow: Workflow): DeleteResponse { | ||
| return client.suspendUntil { delete(deleteRequest, it) } |
There was a problem hiding this comment.
how are we handling failure in deletion?
There was a problem hiding this comment.
add debug logs "deleted monitor $monitorId"
There was a problem hiding this comment.
how are we handling failure in deletion?
Well we are calling delete(deleteRequest, it) as a lambda. You can see that second parameter of delete function call is it -> which is ActionListener forwarded to a delete function.
Something similar we are doing here
| suspend fun validateRequest(request: IndexWorkflowRequest, listener: ActionListener<IndexWorkflowResponse>) { | ||
| val compositeInput = request.workflow.inputs.get(0) as CompositeInput | ||
| suspend fun validateRequest(request: IndexWorkflowRequest) { | ||
| val compositeInput = request.workflow.inputs[0] as CompositeInput |
There was a problem hiding this comment.
lets first validate that the input is a composite input.
| assertEquals("Delegate1 id not correct", monitorResponse1.id, delegate1.monitorId) | ||
| } | ||
|
|
||
| fun `test get workflow`() { |
There was a problem hiding this comment.
test get workflow for invalid id?
| assertEquals("Delegate id not correct", monitorResponse.id, delegate.monitorId) | ||
| } | ||
|
|
||
| fun `test delete workflow`() { |
There was a problem hiding this comment.
test delete workflow for invalid id
| } | ||
| } | ||
|
|
||
| fun `test create workflow duplicate delegate failure`() { |
There was a problem hiding this comment.
test update for all these failure cases
| } | ||
| } | ||
|
|
||
| fun `test create workflow chained findings order not correct failure`() { |
There was a problem hiding this comment.
test delete monitor before workflow is deleted and this monitor is delegate in that sequence
There was a problem hiding this comment.
good point. will test that. will also need to update a logic on the side of monitor deletion
Tnx Will do so. Tnx for constructive feedback |
…nitor index is not initialized yet. Added workflow crud test cases Signed-off-by: Stevan Buzejic <[email protected]>
…e workflow Signed-off-by: Stevan Buzejic <[email protected]>
…enario
Signed-off-by: Stevan Buzejic [email protected]
Issue #, if available:
Description of changes:
CheckList:
[ ] Commits are signed per the DCO using --signoff
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.