Implement odo dev --no-commands#6855
Merged
openshift-merge-robot merged 10 commits intoredhat-developer:mainfrom Jun 14, 2023
Merged
Implement odo dev --no-commands#6855openshift-merge-robot merged 10 commits intoredhat-developer:mainfrom
odo dev --no-commands#6855openshift-merge-robot merged 10 commits intoredhat-developer:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
✅ Deploy Preview for odo-docusaurus-preview ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
openshift-ci
bot
added
the
kind/feature
|
Skipping CI for Draft Pull Request. |
rm3l
changed the title
odo dev --no-commands
rm3l
force-pushed
the
6569-odo-dev-no-commands
branch
from
af5529c to
f2f0b5f
Compare
feloy
reviewed
Jun 1, 2023
| 4. Synchronize the local files to the Dev environment | ||
| 5. Start the port forwarding logic | ||
|
|
||
| Note that this is the default behavior of `odo dev` if the Devfile does not define any commands at all. |
Contributor
There was a problem hiding this comment.
Suggested change
| Note that this is the default behavior of `odo dev` if the Devfile does not define any commands at all. | |
| Note that this is the default behavior of `odo dev` if the Devfile does not define any Build, Run or Debug commands. |
Member
Author
There was a problem hiding this comment.
When there is only one command of a given kind, odo currently automatically uses it even if it is not explicitly marked as the default:
odo/pkg/libdevfile/libdevfile.go
Lines 103 to 106 in b6c9c88
Do we want to change that behavior?
That's why I considered limiting this to the case where there are no commands at all, like with the UDI Stack.
Contributor
There was a problem hiding this comment.
OK, you 're right for the Default commands. I replaced my proposal with:
Note that this is the default behavior of `odo dev` if the Devfile does not define any Build, Run or Debug commands.
Member
Author
There was a problem hiding this comment.
Alright, that makes sense. I'll update it. Thanks.
pkg/libdevfile/libdevfile.go
Outdated
| } | ||
|
|
||
| // HasCommands returns whether the Devfile has any commands. | ||
| func HasCommands(devfileData data.DevfileData) (bool, error) { |
Contributor
There was a problem hiding this comment.
We need to check if there are Default commands of kind Build/Run/Debug
Contributor
There was a problem hiding this comment.
We need to check if there are Default commands of kind Build/Run/Debug
pkg/odo/cli/dev/dev.go
Outdated
| func (o *DevOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) error { | ||
| if !o.noCommandsFlag { | ||
| devfileObj := *odocontext.GetEffectiveDevfileObj(ctx) | ||
| hasCommands, err := libdevfile.HasCommands(devfileObj.Data) |
Contributor
There was a problem hiding this comment.
I think it would be more robust and adaptable if the existence of commands are checked separately when we try to run them. For example, if no Build command exists but a Run command exists, of the other way.
There is also the case when there is not default command, but the user passes the Build or Run command to consider using --build-command and/or --run-command
3 tasks
rm3l
force-pushed
the
6569-odo-dev-no-commands
branch
from
f2f0b5f to
ffbea2c
Compare
rm3l
force-pushed
the
6569-odo-dev-no-commands
branch
2 times, most recently
from
fce1400 to
8d160d6
Compare
Co-authored-by: Philippe Martin <[email protected]>
For example, it should not be possible to call it alongside '--build-command' or '--run-command', because we won't be able to know what to do.
For consistency with how e.g., the build command works, instead of erroring out, `odo dev` will now simply log the error message and wait for new input.
As suggested in review, this makes it more adaptable. As a consequence, if the default (or single) build/run command does not exist, "odo dev" will not error out immediately; instead, it will behave a bit like "odo dev --no-commands" (by starting, but printing a warning about the missing command). The difference here is that if a run command is added later on during the dev session, "odo dev" will pick it up and run it, but "odo dev --no-commands" will continue to purposely ignore it. The existence of the optional default Build is already checked by the 'libdevfile.Build' command. It does not error out if there is no default (or single) Build command. It errors out only if the specified '--build-command' does not exist in the Devfile. Co-authored-by: Philippe Martin <[email protected]>
…and and if there are ports to forward This prevents: i) displaying the "Waiting for the application to be ready" spinner for nothing, if there are no ports to forward ii) waiting until the timeout of 1m has expired if there was no run/debug command executed, in which case, it is less unlikely that the application ports defined in the Devfile will ever be reachable.
rm3l
force-pushed
the
6569-odo-dev-no-commands
branch
from
8d160d6 to
abd98af
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
rm3l
changed the title
odo dev --no-commandsodo dev --no-commands
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
openshift-ci
bot
added
the
lgtm
Member
Author
Issue with /override Kubernetes-Integration-Tests/Kubernetes-Integration-Tests |
|
@rm3l: Overrode contexts on behalf of rm3l: Kubernetes-Integration-Tests/Kubernetes-Integration-Tests DetailsIn response to this:
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. |
Member
Author
Issues with volume mounting on OpenShift. /override OpenShift-Integration-tests/OpenShift-Integration-tests |
|
@rm3l: Overrode contexts on behalf of rm3l: OpenShift-Integration-tests/OpenShift-Integration-tests DetailsIn response to this:
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this:
/kind feature
/area dev
What does this PR do / why we need it:
This adds a new
--no-commandsflag to thedevsubcommand. When this flag is set, nobuild,runordebugcommands are executed. More details in https://deploy-preview-6855--odo-docusaurus-preview.netlify.app/docs/command-reference/dev#running-with-no-commandsThis is useful in conjunction with the
odo runcommand.This PR also changes the behavior of
odo devwhen the Devfile has no (default or single non-default)build,runordebugcommands defined. For more consistency, instead of erroring out at CLI validation time, theodo devsession starts and just prints a warning in this case. For consistency (e.g., when the build command fails), it will just wait for new input and retrigger the inner loop logic.Which issue(s) this PR fixes:
Fixes #6569
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
It should work the same way regardless of the target platform, e.g.:
You can also try with the Universal Developer Image Stack, which only defines a container component and no commands. Previously,
odo devwould error out immediately, but now, it should not:odo dev
odo dev --run-command 404
odo dev --no-commands