MTA Assessment WF #354

rgolangh · 2023-05-18T16:38:33Z

MTA Assessment Workflow

This is an assessment workflow that is based on the Migration Analysis
infra workflow.
The main difference is that the checker of that workflow downloads the
html report and tries to extract a table of scores of issues found by
the analysis. The ProcessAnalysisTask has a predicate passed in the
constructor to test against the scores of the analysis and based on that
the task returns the passOption or the defaultOption. The passOption is
the next flow to invoke, that should be the move2kube.

The flow is roughly:

Start
  -
Infra flow start
  -
submit analysis report
  -
End

Start
  -
Checker flow start
  -
check report is ready
  -
switch (is ready?)
  yes -> go to parse
  no  -> go to check report is ready
  -
parse
  -
switch (match incidents on predicate)
  true - return pass option (move2kube)
  false - return default option
  -
End

.prettierrc.json

RichardW98 · 2023-06-02T20:31:07Z

...src/main/java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/ProcessAnalysisTask.java

+		// this is the "migration issues" section from report from url.It assumed to be in
+		// HTML format. In version 6.1 of MTA it can be
+		// downloaded as csv and would be simpler to process, hopefully.
+		URI uri = URI.create(reportURL).resolve("reports");


should be "report"

I actually needed to add a / in the end. but it works

RichardW98 · 2023-06-02T20:33:40Z

...src/main/java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/ProcessAnalysisTask.java

+				return new DefaultWorkReport(WorkStatus.FAILED, workContext,
+						new Exception("failed to parse report index. Missing reports/report_index_*"));
+			}
+			String migrationIssuesReport = downloadReport(uri.resolve(reportLinks.get(0).text()));


reportLinks.get(0) is <a href="reports/report_index_demo.html" style="float: left; margin-right: 5px; margin-top: 6px;"> *** </a>, I think we only want "reports/report_index_demo.html"

like mentioned on chat - I dropped the html rendering in favour of parsing the javascript data. all this code is gone now

RichardW98 · 2023-06-02T20:37:07Z

.../java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/MigrationAssessmentWorkflow.java

+	}
+
+	@Bean(name = "AnalyzeApplicationAssessment")
+	@Assessment


should repoUrl and appName parameters be here?

yeah I set them as workflow level params

.../java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/MigrationAssessmentWorkflow.java

pkliczewski · 2023-06-05T09:43:38Z

@rgolangh can we please finalize the PR this week?

Signed-off-by: Roy Golan <rgolan@redhat.com>

masayag · 2023-06-06T18:17:54Z

prebuilt-tasks/src/main/java/com/redhat/parodos/tasks/migrationtoolkit/GetAnalysisTask.java

+		request.setMessageType("text");
+		request.setBody("Report URL: " + reportURL);
+		request.subject("Migration Analysis Report Completed");
+		request.setUsernames(List.of("test"));


Shouldn't this be the user that invoked the workflow instead of the default "test" value?
can be obtained by SecurityUtils.getUsername()

masayag · 2023-06-06T18:18:36Z

prettierrc.json

@@ -0,0 +1 @@
+{}


what is this file used for?

that is a reminder to me that I didn't git ignore that yet!

masayag · 2023-06-06T18:19:19Z

.../main/java/com/redhat/parodos/examples/ocponboarding/OcpOnboardingWorkFlowConfiguration.java


 @Configuration
+@Profile("local")


what is the reason to state a profile as part of the workflow-definition?

masayag · 2023-06-06T18:21:54Z

.../main/java/com/redhat/parodos/examples/ocponboarding/OcpOnboardingWorkFlowConfiguration.java

@@ -76,15 +78,24 @@ WorkFlowOption notSupportOption() {
 						.setDescription("Non-Supported Workflow Steps").build();
 	}

+	@Bean
+	WorkFlowOption analyzeOption() {
+		return new WorkFlowOption.Builder("analyzeOption", "AnalyzeApplicationAssessment")


I'd like to suggest adding a class named AnalysysConstants or MTAConstants that will list the constants used here and reduces the chances of misspelling the keys.
We don't have such a convention yet, but we have that as part of Move2kube and I find it useful.

masayag · 2023-06-06T18:24:06Z

...s/src/main/java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/MTAAnalysisReport.java

+public class MTAAnalysisReport {
+
+	/**
+	 * migrationIssuesReport is assumed to be valid html. MTA 6.1 should supply a CSV as


the comment refers to migrationIssuesReport but it is nowhere to be found in this PR.
could you update the comment and perhaps use javadoc style?

masayag · 2023-06-06T18:50:05Z

...in/java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/MigrationAnalysisWorkflow.java

@@ -81,12 +71,16 @@ public SubmitAnalysisTask submitAnalysisTask(WorkFlow fetchReportURL) {
 	}

 	@Bean
-	public GetAnalysisTask getAnalysisTask() {
-		return new GetAnalysisTask(URI.create(mtaUrl), "", messageConsumer());
+	public GetAnalysisTask getAnalysisTask(Notifier notifier) {


+100 for the Notifier. much more elegant and reusable.

masayag · 2023-06-06T18:51:09Z

.../java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/MigrationAssessmentWorkflow.java

+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Profile;
+
+import static com.redhat.parodos.workflows.workflow.SequentialFlow.Builder.aNewSequentialFlow;


no static imports should be allowed in the code. weird the checkstyle didn't fail for it.
it should be allowed only in tests (unless workflow-examples are excluded).

https://github.com/spring-projects/spring-framework/wiki/Code-Style#import-statements

Static imports should not be used in production code, but they should be used in test code, especially for things like import static org.assertj.core.api.Assertions.assertThat;.

workflow-example is considered to be production code

masayag · 2023-06-06T18:51:48Z

.../java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/MigrationAssessmentWorkflow.java

+ * An assessment workflow to analyze applications using Migration Toolkit for Applications
+ * and return a move2kube option when needed.
+ * <p>
+ * This workflow will: - create an application with a name, and git repo URL - submit an


perhaps use
tags to split into bullets to improve readability?

masayag · 2023-06-06T18:53:20Z

.../java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/MigrationAssessmentWorkflow.java

+	}
+
+	@Bean("fetchReportURL")
+	@Checker(cronExpression = "*/5 * * * * ?")


is 5s reasonable or should be a higher value?

masayag · 2023-06-06T18:56:44Z

...src/main/java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/ProcessAnalysisTask.java

+	public WorkReport execute(WorkContext workContext) {
+		String reportURL;
+		try {
+			reportURL = getRequiredParameterValue("reportURL") + "/";


is there a need to check the URL isn't already terminated by "/" ?

This is an assessment workflow which is based on the Migration Analysis infra workflow. The main differne is that the checker of that workflow downloads the html report and tries to extract a table of scores of issues found by the analysis. The ProcessAnalysisTask has a predicate passed in the constructor to test against the scores of the analysis and based on that the task returns the passOption or the defaultOption. The passOption is the next flow to invoke, that should be the move2kube. The flow is roughly: Start - Infra flow start - submit analysis report - End Start - Checker flow start - check report is ready - switch (is ready?) yes -> go to parse no -> go to check report is ready - parse - switch (match incidents on predicate) true - return pass option (move2kube) false - return default option - End Signed-off-by: Roy Golan <rgolan@redhat.com>

openshift-ci · 2023-06-06T19:29:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

~~OWNERS~~ [RichardW98]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2023-06-06T19:29:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

~~OWNERS~~ [RichardW98]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot requested review from nirarg and pkliczewski May 18, 2023 16:38

rgolangh changed the title ~~mta assessment wf~~ WIP: mta assessment wf May 18, 2023

openshift-ci bot added the do-not-merge/work-in-progress label May 18, 2023

rgolangh force-pushed the mta-assessment-wf branch from 3598775 to d8c74ed Compare May 31, 2023 09:29

openshift-merge-robot added the needs-rebase label May 31, 2023

rgolangh force-pushed the mta-assessment-wf branch 3 times, most recently from 24e0127 to ee17910 Compare June 2, 2023 13:21

rgolangh changed the title ~~WIP: mta assessment wf~~ MTA Assessment WF Jun 2, 2023

openshift-ci bot removed the do-not-merge/work-in-progress label Jun 2, 2023

rgolangh force-pushed the mta-assessment-wf branch from ee17910 to 2c137ed Compare June 2, 2023 13:24

RichardW98 reviewed Jun 2, 2023

View reviewed changes

.prettierrc.json Outdated Show resolved Hide resolved

RichardW98 reviewed Jun 2, 2023

View reviewed changes

.../java/com/redhat/parodos/examples/prebuilt/migrationtoolkit/MigrationAssessmentWorkflow.java Outdated Show resolved Hide resolved

rgolangh force-pushed the mta-assessment-wf branch from 2c137ed to 22a78fd Compare June 6, 2023 07:23

openshift-merge-robot removed the needs-rebase label Jun 6, 2023

rgolangh force-pushed the mta-assessment-wf branch 2 times, most recently from d58e4cd to 134913f Compare June 6, 2023 16:43

prebuilt: Better name for the default message sender

b44ac9b

Signed-off-by: Roy Golan <rgolan@redhat.com>

masayag reviewed Jun 6, 2023

View reviewed changes

rgolangh force-pushed the mta-assessment-wf branch from 134913f to 287a7ce Compare June 6, 2023 19:00

rgolangh force-pushed the mta-assessment-wf branch from 287a7ce to e67fa39 Compare June 6, 2023 19:11

RichardW98 approved these changes Jun 6, 2023

View reviewed changes

openshift-ci bot added the approved label Jun 6, 2023

RichardW98 added the lgtm label Jun 6, 2023

openshift-merge-robot merged commit 222c5e9 into rhdhorchestrator:main Jun 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MTA Assessment WF #354

MTA Assessment WF #354

rgolangh commented May 18, 2023 •

edited by masayag

Loading

RichardW98 Jun 2, 2023

rgolangh Jun 6, 2023

RichardW98 Jun 2, 2023 •

edited

Loading

rgolangh Jun 6, 2023

RichardW98 Jun 2, 2023

rgolangh Jun 6, 2023

pkliczewski commented Jun 5, 2023

masayag Jun 6, 2023

masayag Jun 6, 2023

rgolangh Jun 6, 2023

masayag Jun 6, 2023

masayag Jun 6, 2023

masayag Jun 6, 2023

masayag Jun 6, 2023

masayag Jun 6, 2023

rgolangh Jun 7, 2023

masayag Jun 7, 2023

masayag Jun 6, 2023

masayag Jun 6, 2023

masayag Jun 6, 2023

openshift-ci bot commented Jun 6, 2023

openshift-ci bot commented Jun 6, 2023


		@Configuration
		@Profile("local")

MTA Assessment WF #354

MTA Assessment WF #354

Conversation

rgolangh commented May 18, 2023 • edited by masayag Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichardW98 Jun 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkliczewski commented Jun 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Jun 6, 2023

openshift-ci bot commented Jun 6, 2023

rgolangh commented May 18, 2023 •

edited by masayag

Loading

RichardW98 Jun 2, 2023 •

edited

Loading