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.

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

Clean up devfile exec package functions #3518

Closed
maysunfaisal opened this issue Jul 8, 2020 · 1 comment · Fixed by #3596
Closed

Clean up devfile exec package functions #3518

maysunfaisal opened this issue Jul 8, 2020 · 1 comment · Fixed by #3596
Assignees
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. area/refactoring Issues or PRs related to code refactoring

Comments

@maysunfaisal
Copy link
Contributor

pkg/exec/devfile.go's ExecuteDevfileRunActionWithoutRestart(), ExecuteDevfileDebugActionWithoutRestart is ~95% similar to their non restart counterparts ExecuteDevfileRunAction, ExecuteDevfileDebugAction

To me it looks like a lot of redundant code and can be massively cleaned up by modifying the prev existing functions to handle non restarts.

Came across this when looking at ways to refactor kube/docker adapter's execDevfile() func in #3015

/area devfile
/kind cleanup

@openshift-ci-robot openshift-ci-robot added area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/cleanup labels Jul 8, 2020
@kadel
Copy link
Member

kadel commented Jul 10, 2020

/kind code-refactoring

metacosm added a commit to metacosm/odo that referenced this issue Jul 17, 2020
metacosm added a commit to metacosm/odo that referenced this issue Jul 21, 2020
metacosm added a commit to metacosm/odo that referenced this issue Jul 21, 2020
metacosm added a commit to metacosm/odo that referenced this issue Jul 22, 2020
metacosm added a commit to metacosm/odo that referenced this issue Jul 22, 2020
metacosm added a commit to metacosm/odo that referenced this issue Jul 27, 2020
metacosm added a commit to metacosm/odo that referenced this issue Aug 3, 2020
metacosm added a commit to metacosm/odo that referenced this issue Aug 6, 2020
metacosm added a commit to metacosm/odo that referenced this issue Aug 6, 2020
openshift-merge-robot pushed a commit that referenced this issue Aug 7, 2020
* refactor: simplify execution of devfile commands

Fixes #3518

* fix: remove mistakenly re-added PlatformAdapter interface

* refactor: move catalog list output to where it's actually needed

This simplifies the package organization by avoiding to import catalog-
related packages in the lower-level machine output package.

* feat: add LoggingClient method on ComponentAdapter interface

* refactor: rename all receivers similarly

* fix: typo

* refactor: ComponentAdapter implements ExecClient, simplify Execute

* fix: typo

* refactor: introduce GenericAdapter to share more code

* chore: remove now unneeded methods

* fix: update from master

* fix: improper code to ExecuteCommand

* fix: properly initialize Client in adapters

* fix: update tests

* chore: add docs [skip ci]

* refactor: expose component info factory methods

* feat: add ExplicitSpinner function

* refactor: simplify adapters using NewMachineEventLoggingClient factory

* refactor: introduce command concept following the composite pattern

Still needs a lot of work to accommodate composite-support changes

* refactor: re-implement command execution using composite commands

* refactor: re-organize command implementations in multiple files

* doc: document New and command

* fix: typo

* fix: change var name

* fix: improper check for composite command

* clean-up: unneeded code

* refactor: rename ExecuteDevfileCommandSynchronously more appropriately

* chore: re-add some tests

* fix: init the ComponentInfoFactories after the GenericAdapter is created

This is needed because the previous implementation resulted in the
componentInfo and supervisorComponentInfo variables capturing
uninitialized adapters and thus causing errors.

* chore: add documentation

* fix: use Errorf instead of Wrapf since there is no error to wrap

* fix: infinite loop

* fix: do not report an error if the supervisor container is not found

* fix: only start the supervisor if we found its container

* fix: remove redundant check

* fix: supervisor commands now use proper component info

Introduced newSupervisor*Command to encapsulate supervisor command
details.

* fix: do not add to composite if supervisor command is nil

* fix: supervisor start/stop should be more like regular simple commands

* fix: add newline at the end of output

* doc: add documentation

[skip ci]

* refactor: rename createFrom to createCommandFrom

* refactor: make sourcePath a const and move it where it's used

* fix: comment out failing tests

See #3685

* fix: typo

* doc: clarify simpleCommand's behavior with respect to commands

[skip ci]

* feat: do not show spinner if message is empty

* fix: do not show spinner for stop command

* fix: remove unneeded fmt.Sprintf call

* chore: update after rebase
cdrage pushed a commit to cdrage/odo that referenced this issue Aug 7, 2020
* refactor: simplify execution of devfile commands

Fixes redhat-developer#3518

* fix: remove mistakenly re-added PlatformAdapter interface

* refactor: move catalog list output to where it's actually needed

This simplifies the package organization by avoiding to import catalog-
related packages in the lower-level machine output package.

* feat: add LoggingClient method on ComponentAdapter interface

* refactor: rename all receivers similarly

* fix: typo

* refactor: ComponentAdapter implements ExecClient, simplify Execute

* fix: typo

* refactor: introduce GenericAdapter to share more code

* chore: remove now unneeded methods

* fix: update from master

* fix: improper code to ExecuteCommand

* fix: properly initialize Client in adapters

* fix: update tests

* chore: add docs [skip ci]

* refactor: expose component info factory methods

* feat: add ExplicitSpinner function

* refactor: simplify adapters using NewMachineEventLoggingClient factory

* refactor: introduce command concept following the composite pattern

Still needs a lot of work to accommodate composite-support changes

* refactor: re-implement command execution using composite commands

* refactor: re-organize command implementations in multiple files

* doc: document New and command

* fix: typo

* fix: change var name

* fix: improper check for composite command

* clean-up: unneeded code

* refactor: rename ExecuteDevfileCommandSynchronously more appropriately

* chore: re-add some tests

* fix: init the ComponentInfoFactories after the GenericAdapter is created

This is needed because the previous implementation resulted in the
componentInfo and supervisorComponentInfo variables capturing
uninitialized adapters and thus causing errors.

* chore: add documentation

* fix: use Errorf instead of Wrapf since there is no error to wrap

* fix: infinite loop

* fix: do not report an error if the supervisor container is not found

* fix: only start the supervisor if we found its container

* fix: remove redundant check

* fix: supervisor commands now use proper component info

Introduced newSupervisor*Command to encapsulate supervisor command
details.

* fix: do not add to composite if supervisor command is nil

* fix: supervisor start/stop should be more like regular simple commands

* fix: add newline at the end of output

* doc: add documentation

[skip ci]

* refactor: rename createFrom to createCommandFrom

* refactor: make sourcePath a const and move it where it's used

* fix: comment out failing tests

See redhat-developer#3685

* fix: typo

* doc: clarify simpleCommand's behavior with respect to commands

[skip ci]

* feat: do not show spinner if message is empty

* fix: do not show spinner for stop command

* fix: remove unneeded fmt.Sprintf call

* chore: update after rebase
@rm3l rm3l added the area/refactoring Issues or PRs related to code refactoring label Jun 16, 2023
@rm3l rm3l added this to odo Project Jun 16, 2023
@rm3l rm3l moved this to Done ✅ in odo Project Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. area/refactoring Issues or PRs related to code refactoring
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants