Skip to content

Fix UX inconsistency when handling commands bound to events#6574

Merged
openshift-merge-robot merged 5 commits intoredhat-developer:mainfrom
rm3l:ux_harmonize_odo_dev_output_when_running_event_commands
Feb 7, 2023
Merged

Fix UX inconsistency when handling commands bound to events#6574
openshift-merge-robot merged 5 commits intoredhat-developer:mainfrom
rm3l:ux_harmonize_odo_dev_output_when_running_event_commands

Conversation

@rm3l
Copy link
Member

@rm3l rm3l commented Feb 6, 2023

What type of PR is this:
/kind bug
/area UX
/area dev

What does this PR do / why we need it:
See #6573.

For the example provided in the issue description, this PR changes the odo dev output from:

 ✓  Executing copy-galleon command "cp -Rf /opt/jboss/container/wildfly/s2i/galleon/galleon-m2-repository /tmp/. && cp -Rf /opt/wildfly /tmp/." on container "wildfly" [960ms]
 ✓  Executing provision-server command "/usr/local/s2i/assemble && cp -Rf $JBOSS_HOME ." on container "wildfly" [8s]
 ✓  Executing store-config command "mkdir $STANDALONE_RESTORE && cp -f $JBOSS_HOME/standalone/configuration/standalone.xml $STANDALONE_RESTORE/standalone.xml" on container "wildfly" [47ms]

to

 ✓  Executing post-start command in container (command: copy-galleon) [835ms]
 ✓  Executing post-start command in container (command: provision-server) [8s]
 ✓  Executing post-start command in container (command: store-config) [52ms]

Which issue(s) this PR fixes:
Fixes #6573

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

$ odo init --name my-java-wildfly --devfile java-wildfly --starter microprofile-openapi
$ odo dev

rm3l added 4 commits February 6, 2023 13:04
@rm3l
Do not display the full command-line when handling exec commands and …
bb6ac79 
…no msg is specified

Displaying it adds a lot of unnecessary clutter.
@rm3l
Display a shorter message when handling commands bound to 'pre-stop' …
2c74791 
…events

This is to be more consistent with how the other messages are displayed.
@rm3l
Display a shorter message when handling commands bound to 'post-start…
13c9c9e 
…' events

This is to be more consistent with how the other messages are displayed.
@rm3l
Fix integration tests
543e043
@netlify
Copy link

netlify bot commented Feb 6, 2023 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 11c02cd
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63e13c08c7a2de000891be9d

@openshift-ci openshift-ci bot added kind/bug Categorizes issue or PR as related to a bug. area/UX Issues or PRs related to User Experience area/dev Issues or PRs related to `odo dev` labels Feb 6, 2023
@openshift-ci openshift-ci bot requested review from anandrkskd and feloy February 6, 2023 12:47
@rm3l rm3l requested review from valaparthvi and removed request for feloy February 6, 2023 12:48
@odo-robot
Copy link

odo-robot bot commented Feb 6, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Feb 6, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Feb 6, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Feb 6, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Feb 6, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Feb 6, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Feb 6, 2023 •
edited
Loading

OpenShift Tests on commit 2bd9cd5 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 6, 2023 •
edited
Loading

Kubernetes Docs Tests on commit 5c38573 finished successfully.
View logs: TXT HTML

anandrkskd
anandrkskd reviewed Feb 6, 2023
View reviewed changes
pkg/component/exec_handler.go
msg := o.msg
if msg == "" {
msg = fmt.Sprintf("Executing %s command %q on container %q", command.Id, command.Exec.CommandLine, command.Exec.Component)
msg = fmt.Sprintf("Executing %s command on container %q", command.Id, command.Exec.Component)
Copy link
Contributor

@anandrkskd anandrkskd Feb 6, 2023

Choose a reason for hiding this comment

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

@rm3l just to understand better, this function is used to execute command inside the container?

Copy link
Member Author

@rm3l rm3l Feb 6, 2023

Choose a reason for hiding this comment

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

Yes, it is an implementation of a generic libdevfile.Handler interface, used to "handle" commands defined in the Devfile that odo needs to take into account, depending on their type (apply, exec, ...).
Here it is the handler for exec commands, and it delegates to an exec client that does the job of executing the command inside the container (via again a generic platform client interface, with different implementations for Podman and K8s).

@rm3l rm3l closed this Feb 6, 2023
@rm3l rm3l reopened this Feb 6, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Feb 7, 2023
@openshift-merge-robot openshift-merge-robot merged commit 162a193 into redhat-developer:main Feb 7, 2023
@rm3l rm3l deleted the ux_harmonize_odo_dev_output_when_running_event_commands branch February 7, 2023 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@anandrkskd anandrkskd anandrkskd left review comments

@valaparthvi valaparthvi valaparthvi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@valaparthvi valaparthvi

Labels

area/dev Issues or PRs related to `odo dev` area/UX Issues or PRs related to User Experience kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. Required by Prow.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

UX inconsistency when handling commands bound to events

4 participants

@rm3l @valaparthvi @anandrkskd @openshift-merge-robot