Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix UX inconsistency when handling commands bound to events #6574

Merged
merged 5 commits into from
Feb 7, 2023
Merged

Fix UX inconsistency when handling commands bound to events #6574

merged 5 commits into from
Feb 7, 2023

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
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
@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
@@ -55,7 +55,7 @@ func (o *execHandler) ApplyOpenShift(openshift v1alpha2.Component) error {
func (o *execHandler) Execute(command v1alpha2.Command) error {
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

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

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
@sonarcloud
Copy link

sonarcloud 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

@anandrkskd anandrkskd anandrkskd left review comments

@valaparthvi valaparthvi valaparthvi approved these changes

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