Speedup odo registry --details by feloy · Pull Request #6678 · redhat-developer/odo

feloy · 2023-03-23T15:04:35Z

What type of PR is this:

/kind feature

What does this PR do / why we need it:

odo registry --details -o json continues to make new calls to get Devfile content for all stacks of registries (to not break plugin compatibility). This command will be deprecated, as --details flag was not intended to be used without --devfile flag.
odo registry --details --devfile "name" -o json continues to make a new call to get the "name" Devfile content (to not break plugin compatibility).
odo registry --details do not make anymore new calls to get Devfile content for all stacks of registries. This command will be deprecated, as --details flag was not intended to be used without --devfile flag.
odo registry --details --devfile "name" do not make anymore a new call to get the "name" Devfile content.

Which issue(s) this PR fixes:

Fixes #6401

PR acceptance criteria:

Unit test
Integration test
Documentation

How to test changes / Special notes to the reviewer:

netlify · 2023-03-23T15:04:53Z

✅ Deploy Preview for odo-docusaurus-preview ready!

Name	Link
🔨 Latest commit	`c347db3`
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/642ab7350c353b000836d42e
😎 Deploy Preview	https://deploy-preview-6678--odo-docusaurus-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

odo-robot · 2023-03-23T15:11:07Z

NoCluster Tests on commit b4a4b10 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-23T15:17:24Z

Unit Tests on commit b4a4b10 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-23T15:19:27Z

Validate Tests on commit b4a4b10 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-23T15:32:12Z

Windows Tests (OCP) on commit b4a4b10 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-23T15:47:26Z

Kubernetes Tests on commit b4a4b10 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-23T15:58:54Z

OpenShift Tests on commit b4a4b10 finished with errors.
View logs: TXT HTML

valaparthvi

Changes lgtm, and the speed is visible. One small change is to remove reference to devfileData from L132.

rm3l

/hold

A side-effet is that the stack content is not available anymore in the JSON output of the command. This removal will break the IDE plugins.

Indeed, I think we'll need to check with the IDE plugins or find a way not to break the output structure..

After a quick search into the source code, the VSCode plugin seems to rely on the .devfileData.devfile field from the odo registry --details output.
See:

feloy · 2023-03-24T16:15:25Z

/hold

A side-effet is that the stack content is not available anymore in the JSON output of the command. This removal will break the IDE plugins.

Indeed, I think we'll need to check with the IDE plugins or find a way not to break the output structure..

After a quick search into the source code, the VSCode plugin seems to rely on the .devfileData.devfile field from the odo registry --details output. See:

https://github.com/redhat-developer/vscode-openshift-tools/blob/main/src/registriesView.ts#L141-L151

https://github.com/redhat-developer/vscode-openshift-tools/blob/main/src/odo/command.ts#L285-L294

Yes, right. These lines particularly are using the supportedOdoFeatures which are removed by the current patch.

https://github.com/redhat-developer/vscode-openshift-tools/blob/main/src/openshift/component.ts#L108-L110

Also the devfileData.devfile.metadata.name used in many places, for example
https://github.com/redhat-developer/vscode-openshift-tools/blob/main/src/openshift/component.ts#L386

odo-robot · 2023-03-28T07:15:31Z

OpenShift Unauthenticated Tests on commit b4a4b10 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-28T07:51:53Z

Kubernetes Docs Tests on commit bc0c023 finished with errors.
View logs: TXT HTML

feloy · 2023-03-28T08:07:10Z

~~@rm3l Here is my proposal to try and fix the issue about compatibility with plugins: 17bbda1~~

feloy · 2023-03-28T13:18:36Z

I've made changes according to what has been discussed during cabal. See #6678 (comment) for more details.

feloy · 2023-04-03T10:04:06Z

/override windows-integration-test/Windows-test
Flaky test

openshift-ci · 2023-04-03T10:04:11Z

@feloy: Overrode contexts on behalf of feloy: windows-integration-test/Windows-test

Details

In response to this:

/override windows-integration-test/Windows-test
Flaky test

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.

docs/website/docs/command-reference/json-output.md

sonarqubecloud · 2023-04-03T11:24:20Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

rm3l · 2023-04-03T11:59:45Z

Sounds good to me to remove the hold on this PR, now that we are pretty confident in not breaking the IDE plugins, no? What do you think?
I added a reminder to document the deprecated odo registry --details (without --devfile) in the v3.9.0 release notes too.

feloy · 2023-04-03T12:24:20Z

Sounds good to me to remove the hold on this PR, now that we are pretty confident in not breaking the IDE plugins, no? What do you think? I added a reminder to document the deprecated odo registry --details (without --devfile) in the v3.9.0 release notes too.

Yes, the JSON output is not changed by this patch, it should not break the IDE plugins

rm3l · 2023-04-03T12:37:57Z

Sounds good to me to remove the hold on this PR, now that we are pretty confident in not breaking the IDE plugins, no? What do you think? I added a reminder to document the deprecated odo registry --details (without --devfile) in the v3.9.0 release notes too.

Yes, the JSON output is not changed by this patch, it should not break the IDE plugins

Thanks for confirming.

/hold cancel

feloy · 2023-04-03T13:24:38Z

/override OpenShift-Integration-tests/OpenShift-Integration-tests

Flaky e2e test

 [FAILED] [212.271 seconds]
E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
/go/odo_1/tests/e2escenarios/e2e_test.go:337

openshift-ci · 2023-04-03T13:24:42Z

@feloy: Overrode contexts on behalf of feloy: OpenShift-Integration-tests/OpenShift-Integration-tests

Details

In response to this:

/override OpenShift-Integration-tests/OpenShift-Integration-tests

Flaky e2e test

[FAILED] [212.271 seconds]
E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
/go/odo_1/tests/e2escenarios/e2e_test.go:337

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.

openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Mar 23, 2023

openshift-ci bot requested review from rm3l and valaparthvi March 23, 2023 15:04

feloy force-pushed the feature-6401/details branch from 56f5912 to 5aaa62f Compare March 23, 2023 15:56

feloy force-pushed the feature-6401/details branch from 5aaa62f to 987d6f6 Compare March 23, 2023 17:03

feloy changed the title ~~Speedup odo registry --details~~ [WIP] Speedup odo registry --details Mar 24, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Mar 24, 2023

valaparthvi suggested changes Mar 24, 2023

View reviewed changes

openshift-ci bot assigned valaparthvi Mar 24, 2023

rm3l requested changes Mar 24, 2023

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Mar 24, 2023

openshift-ci bot assigned rm3l Mar 24, 2023

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Mar 25, 2023

feloy force-pushed the feature-6401/details branch from 987d6f6 to 24b378e Compare March 28, 2023 07:09

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Mar 28, 2023

feloy force-pushed the feature-6401/details branch from 17bbda1 to ce11905 Compare March 28, 2023 13:03

feloy changed the title ~~[WIP] Speedup odo registry --details~~ Speedup odo registry --details Mar 28, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Mar 28, 2023

feloy added 4 commits March 31, 2023 18:42

Fix integration tests

04564fe

Use CommandGroups from registry index

8c1abb8

Fix JSON field name / Detailed err message

5d68486

Clarify --details is to be used with --devfile

dcd0629

feloy force-pushed the feature-6401/details branch from ab0407e to dcd0629 Compare March 31, 2023 16:42

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 31, 2023

feloy requested a review from rm3l March 31, 2023 16:43

feloy closed this Apr 3, 2023

feloy reopened this Apr 3, 2023

rm3l approved these changes Apr 3, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 3, 2023

rm3l reviewed Apr 3, 2023

View reviewed changes

docs/website/docs/command-reference/json-output.md Show resolved Hide resolved

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 3, 2023

feloy requested a review from rm3l April 3, 2023 10:58

rm3l mentioned this pull request Apr 3, 2023

🎉 Release for Sprints 233 through 234 (v3.9.0) 🎉 #6643

Closed

feloy force-pushed the feature-6401/details branch from 5c90fdf to 02a0236 Compare April 3, 2023 11:20

Document versions.commandGroups in json output

c347db3

feloy force-pushed the feature-6401/details branch from 02a0236 to c347db3 Compare April 3, 2023 11:23

rm3l approved these changes Apr 3, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 3, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Apr 3, 2023

openshift-merge-robot merged commit 252a77d into redhat-developer:main Apr 3, 2023

Conversation

feloy commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for odo-docusaurus-preview ready!

Uh oh!

odo-robot bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odo-robot bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odo-robot bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odo-robot bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odo-robot bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odo-robot bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valaparthvi left a comment

Choose a reason for hiding this comment

Uh oh!

rm3l left a comment

Choose a reason for hiding this comment

Uh oh!

feloy commented Mar 24, 2023

Uh oh!

odo-robot bot commented Mar 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odo-robot bot commented Mar 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feloy commented Mar 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feloy commented Mar 28, 2023

Uh oh!

feloy commented Apr 3, 2023

Uh oh!

openshift-ci bot commented Apr 3, 2023

Uh oh!

Uh oh!

sonarqubecloud bot commented Apr 3, 2023

Uh oh!

rm3l commented Apr 3, 2023

Uh oh!

feloy commented Apr 3, 2023

Uh oh!

rm3l commented Apr 3, 2023

Uh oh!

feloy commented Apr 3, 2023

Uh oh!

openshift-ci bot commented Apr 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feloy commented Mar 23, 2023 •

edited

Loading

netlify bot commented Mar 23, 2023 •

edited

Loading

odo-robot bot commented Mar 23, 2023 •

edited

Loading

odo-robot bot commented Mar 23, 2023 •

edited

Loading

odo-robot bot commented Mar 23, 2023 •

edited

Loading

odo-robot bot commented Mar 23, 2023 •

edited

Loading

odo-robot bot commented Mar 23, 2023 •

edited

Loading

odo-robot bot commented Mar 23, 2023 •

edited

Loading

odo-robot bot commented Mar 28, 2023 •

edited

Loading

odo-robot bot commented Mar 28, 2023 •

edited

Loading

feloy commented Mar 28, 2023 •

edited

Loading