Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRAFT - WIP: Add api to monitor workflows. #139

Merged
merged 8 commits into from
Sep 12, 2022

Conversation

symphony-soufiane
Copy link
Contributor

Description

This is a draft proposal and a WIP. 2 apis completely covered in this first commit: list all workflows and list all instances for a given workflow.
I started implementing the third api to list all activities for a given workflow instance, some problematics related to the variables, activity type will be fixed in next commits.

WIP:
This is a draft proposal and a WIP. 2 apis completely covered in this first commit: list all workflows and list all instances for a given workflow.
I started implementing the third api to list all activities for a given workflow instance, some problematics related to the veriables, activity type will be fixed in next commits.
@@ -4,7 +4,7 @@ plugins {

group = 'org.finos.symphony.wdk'

ext.projectVersion = '1.2.0-SNAPSHOT'
ext.projectVersion = '1.3.0-SNAPSHOT'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should not be committed, it comes from #138 that still needs to be merged.

@@ -26,7 +26,7 @@ public class ActivityExpiredNodeBuilder extends ActivityNodeBuilder {
builder = context.removeLastEventSubProcessBuilder().subProcessDone();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes in folder bpmn/builder are not related to this PR purpose, just removing warnings for an unused parameter..

import java.util.stream.Collectors;

@RestController
@RequestMapping("v1/monitoring/workflows")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i feel that /monitoring is not necessary.

this.monitoringService = monitoringService;
}

@GetMapping("/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add swagger annotations to describe our api ?

private String instanceId;
private String activityId;
private String type; //TODO: enum needed here
private InstanceStatusEnum status;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the instance status inside an activity ?

Copy link
Contributor Author

@symphony-soufiane symphony-soufiane Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the activity instance status. An activity has instance like the process. InstanceStatusEnum is used for both the workflow process instance view and the activity view.
Since we always get this info from the historicService, it will always be "COMPLETED" so maybe we can get ride of it. Activity instances with ongoing execution should appear in the runtimeService, but they either stay there for a very short time or they are not even set there because the database is updated when no work is ongoing (maybe Camunda updates database in a batch or the process is synchronous..)

I can also change the class name to StatusEnum so it does not reflect the instance aspect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

statusEnum sounds good, avoid the confusion between process instance and activity instance


@RequiredArgsConstructor
@Component
public class MonitoringService {
Copy link
Contributor

@yinan-symphony yinan-symphony Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to create first an interface, a DAO interface would be even better

@symphony-soufiane symphony-soufiane merged commit b9ba648 into finos:master Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants