-
Notifications
You must be signed in to change notification settings - Fork 27
Conversation
@@ -66,7 +66,7 @@ WorkFlow assessmentWorkFlow( | |||
@Qualifier("onboardingAssessmentTask") OnboardingAssessmentTask onboardingAssessmentTask) { | |||
// @formatter:off | |||
return SequentialFlow.Builder.aNewSequentialFlow() | |||
.named("onboarding Assessment WorkFlow") | |||
.named("onboardingAssessment" + WorkFlowConstants.ASSESSMENT_WORKFLOW) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A question about this one: Isn't it redundant to specify the workflow type by annotation and also by the name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question Moti! its name is just a metadata and it should be consistent from bean to builder(basically you can give a workflow any name). Annotation is which type it really is, we need to process a workflow based on its annotated type(e.g. assessment should return options, checker should run on a schedule for certain tasks and so on).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me know if it makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@masayag Workflow type in parados is either Infrastructure, Assessment and Checker. Annotations have been introduced to make that distinction including the need for additional metadata. On the other hand, workflow processing type (Sequential or Parallel) is derived form the workflow bean. Hope that helps clarifying the workflow type part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Workflow type in parados is either Infrastructure, Assessment and Checker. Annotations
and Escalation since the weekend ;-)
Thank you both for your replies. That makes sense to me.
/lgtm |
@masayag: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gciavarrini, RichardW98 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fix workflow name
workflow name should be consistent with its bean name for aspect discovery