Replace odo analyze command #4330

msivasubramaniaan · 2024-07-30T17:41:56Z

Below are the things missed and I don't know how to handle below two items. Let me know if you have any idea.
https://github.com/devfile/alizer doesn't provide any checksum on the release, So the CLI on our extension downloaded each time
alizer devfile command not having "name" key in the response. But odo analyze does
Ex: {
devfile: "java-maven",
devfileRegistry: "Test1",
devfileVersion: "1.3.1",
name: "org-eclipse-lemminx",
}

…ers) text Signed-off-by: msivasubramaniaan <[email protected]>

Signed-off-by: msivasubramaniaan <[email protected]>

codecov-commenter · 2024-07-30T17:54:05Z

Codecov Report

Attention: Patch coverage is 66.66667% with 11 lines in your changes missing coverage. Please review.

Project coverage is 43.94%. Comparing base (da60441) to head (7b2a13b).
Report is 439 commits behind head on main.

Files	Patch %	Lines
.../webview/create-component/createComponentLoader.ts	42.85%	8 Missing ⚠️
src/downloadUtil/downloadBinaries.ts	50.00%	2 Missing ⚠️
src/alizer/alizerWrapper.ts	91.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #4330       +/-   ##
===========================================
+ Coverage   32.37%   43.94%   +11.57%     
===========================================
  Files          85       96       +11     
  Lines        6505     7759     +1254     
  Branches     1349     1660      +311     
===========================================
+ Hits         2106     3410     +1304     
+ Misses       4399     4349       -50

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

datho7561 · 2024-07-30T20:44:40Z

I still don't think I get the point of this PR. If we are removing odo completely, then we won't be able to work with Devfiles, so we won't need to analyze which one to use.

I understand using alizer instead of odo for #4235 , but that's not what this is.

msivasubramaniaan · 2024-08-01T13:30:02Z

I still don't think I get the point of this PR. If we are removing odo completely, then we won't be able to work with Devfiles, so we won't need to analyze which one to use.

I understand using alizer instead of odo for #4235 , but that's not what this is.

Hello @datho7561
As this PR for just replace odo analyze command with alizer devfile. It is common for both component creation using git import and #4235

datho7561

From my manual testing, this seems good. I noticed two small things in the code.

datho7561 · 2024-08-01T15:00:15Z

src/odo/odoWrapper.ts

-        const parse = JSON.parse(cliData.stdout) as AnalyzeResponse[];
-        return parse;
+        const parse = JSON.parse(cliData.stdout) as AlizerAnalyzeResponse[];
+        return [[...parse].shift()];


Shift returns the first element. Why not just return the first element (or undefined if there is no element) and change the return type to Promise<AlizerAnalyzeResponse> ?

datho7561 · 2024-08-01T15:12:12Z

src/odo/odoWrapper.ts

@@ -194,13 +194,12 @@ export class Odo {
        );
    }

-    public async analyze(currentFolderPath: string): Promise<AnalyzeResponse[]> {
+    public async alizerAnalyze(currentFolderPath: Uri): Promise<AlizerAnalyzeResponse[]> {


It might be a good idea to move this function to it's own file, since it no longer uses odo, and this file contains wrappers around odo commands

Signed-off-by: msivasubramaniaan <[email protected]>

vrubezhny

odoWrapper test also needs some attention.

vrubezhny · 2024-08-02T14:55:58Z

test/integration/odoWrapper.test.ts

@@ -100,12 +101,12 @@ suite('./odo/odoWrapper.ts', function () {
        });

        test('analyze()', async function () {


@msivasubramaniaan The test('analyze()', async function () { test case should probably also be moved out from test/integration/odoWrapper.test.ts into a separate test, shouldn't it?

Signed-off-by: msivasubramaniaan <[email protected]>

vrubezhny · 2024-08-10T21:23:54Z

@msivasubramaniaan The related test are still failing:

1) Create component from git URL
2) "after each" hook: context for "Create component from git URL"
3) "after all" hook: context for "Create component from template project"

The first one looks like is hanging out or take a very long time to complete, causing the following tests to fail too. But it could be also that some alizer related code failure causes it not to finish successfully.

Could you please take a look?

Signed-off-by: msivasubramaniaan <[email protected]>

msivasubramaniaan · 2024-08-13T11:09:24Z

@datho7561 and @vrubezhny Moved odo analyze command and corresponding test cases as well. Please review and merge. Once this merged I will make the #4235 PR ready

vrubezhny

Looks good to me. Thanks!

test/integration/alizerWrapper.test.ts

datho7561 · 2024-08-13T12:37:41Z

src/webview/create-component/createComponentLoader.ts

-                res.devfileVersion === version &&
-                res.devfileRegistry === registry.name,
-        ),
+    return Array.from(compDescriptions).filter(({ name, version }) => {


if (foo) { return true; } return false;

is the same as

return foo;

If you find the first version easier to understand feel free to keep it, but I personally tend to prefer the second version.

Signed-off-by: msivasubramaniaan <[email protected]>

datho7561

Looks good to me. Thanks, Muthu!

msivasubramaniaan added 3 commits July 25, 2024 19:37

chaged the image path and removed (formerly Red Hat CodeReady Contain…

3f34d5c

…ers) text Signed-off-by: msivasubramaniaan <[email protected]>

removed odo analyze command

23e0616

Signed-off-by: msivasubramaniaan <[email protected]>

Merge branch 'main' into added-alizer-analyze-command

1b01f49

msivasubramaniaan requested review from vrubezhny and datho7561 July 30, 2024 17:41

msivasubramaniaan self-assigned this Jul 30, 2024

msivasubramaniaan changed the title ~~Added alizer analyze command~~ Replace odo analyze command Jul 30, 2024

datho7561 requested changes Aug 1, 2024

View reviewed changes

addressed review comments

6338174

Signed-off-by: msivasubramaniaan <[email protected]>

msivasubramaniaan requested a review from datho7561 August 2, 2024 09:16

vrubezhny requested changes Aug 2, 2024

View reviewed changes

msivasubramaniaan added 8 commits August 8, 2024 21:36

added alizer test

8a9264e

Signed-off-by: msivasubramaniaan <[email protected]>

fixed no-unused-vars error

f8dcc69

Signed-off-by: msivasubramaniaan <[email protected]>

fixed no-unused-vars error

71031a3

Signed-off-by: msivasubramaniaan <[email protected]>

fixed no-unused-vars error

265507d

Signed-off-by: msivasubramaniaan <[email protected]>

fixed no-unused-vars error

e42fd0c

Signed-off-by: msivasubramaniaan <[email protected]>

fixed no-unused-vars error

3be7b53

Signed-off-by: msivasubramaniaan <[email protected]>

fixed no-unused-vars error

c614dd5

Signed-off-by: msivasubramaniaan <[email protected]>

fixed test case fails

6a3ca0b

Signed-off-by: msivasubramaniaan <[email protected]>

msivasubramaniaan requested a review from vrubezhny August 8, 2024 17:15

msivasubramaniaan added 2 commits August 13, 2024 15:56

added deleteComponentConfiguration

e105c2d

Signed-off-by: msivasubramaniaan <[email protected]>

fixed mapping component description

ddbb56b

Signed-off-by: msivasubramaniaan <[email protected]>

vrubezhny previously approved these changes Aug 13, 2024

View reviewed changes

datho7561 requested changes Aug 15, 2024

View reviewed changes

Merge branch 'main' into added-alizer-analyze-command

f57469b

addressed review comments

c82fa22

Signed-off-by: msivasubramaniaan <[email protected]>

msivasubramaniaan dismissed vrubezhny’s stale review via c82fa22 August 16, 2024 08:59

msivasubramaniaan requested a review from datho7561 August 16, 2024 08:59

addressed review comments

7b2a13b

Signed-off-by: msivasubramaniaan <[email protected]>

msivasubramaniaan requested a review from vrubezhny August 16, 2024 09:00

datho7561 approved these changes Aug 16, 2024

View reviewed changes

datho7561 merged commit 44ebc51 into redhat-developer:main Aug 16, 2024
4 checks passed

msivasubramaniaan deleted the added-alizer-analyze-command branch August 16, 2024 14:31

vrubezhny added this to the 1.16.0 milestone Aug 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace odo analyze command #4330

Replace odo analyze command #4330

msivasubramaniaan commented Jul 30, 2024 •

edited

Loading

codecov-commenter commented Jul 30, 2024 •

edited

Loading

datho7561 commented Jul 30, 2024

msivasubramaniaan commented Aug 1, 2024

datho7561 left a comment

datho7561 Aug 1, 2024

datho7561 Aug 1, 2024

vrubezhny left a comment

vrubezhny Aug 2, 2024

vrubezhny commented Aug 10, 2024 •

edited

Loading

msivasubramaniaan commented Aug 13, 2024

vrubezhny left a comment

datho7561 Aug 13, 2024

datho7561 left a comment

		@@ -100,12 +101,12 @@ suite('./odo/odoWrapper.ts', function () {
		});

		test('analyze()', async function () {

Replace odo analyze command #4330

Replace odo analyze command #4330

Conversation

msivasubramaniaan commented Jul 30, 2024 • edited Loading

codecov-commenter commented Jul 30, 2024 • edited Loading

Codecov Report

datho7561 commented Jul 30, 2024

msivasubramaniaan commented Aug 1, 2024

datho7561 left a comment

Choose a reason for hiding this comment

datho7561 Aug 1, 2024

Choose a reason for hiding this comment

datho7561 Aug 1, 2024

Choose a reason for hiding this comment

vrubezhny left a comment

Choose a reason for hiding this comment

vrubezhny Aug 2, 2024

Choose a reason for hiding this comment

vrubezhny commented Aug 10, 2024 • edited Loading

msivasubramaniaan commented Aug 13, 2024

vrubezhny left a comment

Choose a reason for hiding this comment

datho7561 Aug 13, 2024

Choose a reason for hiding this comment

datho7561 left a comment

Choose a reason for hiding this comment

msivasubramaniaan commented Jul 30, 2024 •

edited

Loading

codecov-commenter commented Jul 30, 2024 •

edited

Loading

vrubezhny commented Aug 10, 2024 •

edited

Loading