Skip to content

Commit

Permalink
Addressed PR comments
Browse files Browse the repository at this point in the history
Signed-off-by: Owais Kazi <[email protected]>
  • Loading branch information
owaiskazi19 committed Nov 9, 2023
1 parent 7a0b6d3 commit b388c7a
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
throw new FlowFrameworkException("workflow_id cannot be null", RestStatus.BAD_REQUEST);
}
// Create request and provision
WorkflowRequest workflowRequest = new WorkflowRequest(workflowId, null, null, null);
WorkflowRequest workflowRequest = new WorkflowRequest(workflowId, null);
return channel -> client.execute(ProvisionWorkflowAction.INSTANCE, workflowRequest, ActionListener.wrap(response -> {
XContentBuilder builder = response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS);
channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<Work

if (request.getWorkflowId() == null) {
// Throttle incoming requests
onSearchGlobalContext(request.getRequestTimeout(), request.getMaxWorkflows(), ActionListener.wrap(max -> {
checkMaxWorkflows(request.getRequestTimeout(), request.getMaxWorkflows(), ActionListener.wrap(max -> {

Check warning on line 114 in src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java#L114

Added line #L114 was not covered by tests
if (!max) {
String errorMessage = "Maximum workflows limit reached " + request.getMaxWorkflows();
logger.error(errorMessage);
Expand Down Expand Up @@ -195,7 +195,7 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<Work
* @param maxWorkflow max workflows
* @param internalListener listener for search request
*/
protected void onSearchGlobalContext(TimeValue requestTimeOut, Integer maxWorkflow, ActionListener<Boolean> internalListener) {
protected void checkMaxWorkflows(TimeValue requestTimeOut, Integer maxWorkflow, ActionListener<Boolean> internalListener) {
QueryBuilder query = QueryBuilders.matchAllQuery();
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(query).size(0).timeout(requestTimeOut);

Check warning on line 200 in src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java#L199-L200

Added lines #L199 - L200 were not covered by tests

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ public class WorkflowRequest extends ActionRequest {
*/
private Integer maxWorkflows;

/**
* Instantiates a new WorkflowRequest, defaults dry run to false and set requestTimeout and maxWorkflows to null
* @param workflowId the documentId of the workflow
* @param template the use case template which describes the workflow
*/
public WorkflowRequest(@Nullable String workflowId, @Nullable Template template) {
this(workflowId, template, false, null, null);
}

/**
* Instantiates a new WorkflowRequest and defaults dry run to false
* @param workflowId the documentId of the workflow
Expand Down Expand Up @@ -76,8 +85,8 @@ public WorkflowRequest(
@Nullable String workflowId,
@Nullable Template template,
boolean dryRun,
TimeValue requestTimeout,
Integer maxWorkflows
@Nullable TimeValue requestTimeout,
@Nullable Integer maxWorkflows
) {
this.workflowId = workflowId;
this.template = template;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void setUp() throws Exception {
threadPool = mock(ThreadPool.class);
settings = Settings.builder()
.put("plugins.flow_framework.max_workflows.", 2)
.put("plugins.anomaly_detection.backoff_initial_delay", TimeValue.timeValueSeconds(10))
.put("plugins.flow_framework.request_timeout", TimeValue.timeValueSeconds(10))
.build();
this.flowFrameworkIndicesHandler = mock(FlowFrameworkIndicesHandler.class);
this.workflowProcessSorter = new WorkflowProcessSorter(mock(WorkflowStepFactory.class), threadPool);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public void testProvisionWorkflow() {
String workflowId = "1";
@SuppressWarnings("unchecked")
ActionListener<WorkflowResponse> listener = mock(ActionListener.class);
WorkflowRequest workflowRequest = new WorkflowRequest(workflowId, null, null, null);
WorkflowRequest workflowRequest = new WorkflowRequest(workflowId, null);

doAnswer(invocation -> {
ActionListener<GetResponse> responseListener = invocation.getArgument(1);
Expand All @@ -124,7 +124,7 @@ public void testProvisionWorkflow() {
public void testFailedToRetrieveTemplateFromGlobalContext() {
@SuppressWarnings("unchecked")
ActionListener<WorkflowResponse> listener = mock(ActionListener.class);
WorkflowRequest request = new WorkflowRequest("1", null, null, null);
WorkflowRequest request = new WorkflowRequest("1", null);
doAnswer(invocation -> {
ActionListener<GetResponse> responseListener = invocation.getArgument(1);
responseListener.onFailure(new Exception("Failed to retrieve template from global context."));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void setUp() throws Exception {
}

public void testNullIdWorkflowRequest() throws IOException {
WorkflowRequest nullIdRequest = new WorkflowRequest(null, template, null, null);
WorkflowRequest nullIdRequest = new WorkflowRequest(null, template);
assertNull(nullIdRequest.getWorkflowId());
assertEquals(template, nullIdRequest.getTemplate());
assertNull(nullIdRequest.validate());
Expand All @@ -71,7 +71,7 @@ public void testNullIdWorkflowRequest() throws IOException {
}

public void testNullTemplateWorkflowRequest() throws IOException {
WorkflowRequest nullTemplateRequest = new WorkflowRequest("123", null, null, null);
WorkflowRequest nullTemplateRequest = new WorkflowRequest("123", null);
assertNotNull(nullTemplateRequest.getWorkflowId());
assertNull(nullTemplateRequest.getTemplate());
assertNull(nullTemplateRequest.validate());
Expand All @@ -87,7 +87,7 @@ public void testNullTemplateWorkflowRequest() throws IOException {
}

public void testWorkflowRequest() throws IOException {
WorkflowRequest workflowRequest = new WorkflowRequest("123", template, null, null);
WorkflowRequest workflowRequest = new WorkflowRequest("123", template);
assertNotNull(workflowRequest.getWorkflowId());
assertEquals(template, workflowRequest.getTemplate());
assertNull(workflowRequest.validate());
Expand Down

0 comments on commit b388c7a

Please sign in to comment.