fix(cli): svc/job delete should select from SSM#2182
Merged
mergify[bot] merged 2 commits intoaws:mainlinefrom Apr 21, 2021
Merged
fix(cli): svc/job delete should select from SSM#2182mergify[bot] merged 2 commits intoaws:mainlinefrom
mergify[bot] merged 2 commits intoaws:mainlinefrom
Conversation
huanjani
approved these changes
Apr 21, 2021
Contributor
huanjani
left a comment
huanjani
left a comment
There was a problem hiding this comment.
Thanks for finding this bug and fixing it!
efekarakus
reviewed
Apr 21, 2021
Contributor
efekarakus
left a comment
efekarakus
left a comment
There was a problem hiding this comment.
LGTM, I think originally we had it only working within a repository so that we delete the copilot/.workspace file.
With these changes, I wonder if we should improve our messages when an application is not found in a workspace 🤔. Right now we display:
✘ get application example: couldn't find an application named example in account 111122223333 and region us-west-2
what do you think of logging some additional context so that the users can unblock themselves?
We detected the application "example" in "copilot/.workspace", to use a different application please delete the file.
✘ get application example: couldn't find an application named example in account 111122223333 and region us-west-2
| log.Infof("Only found one service, defaulting to: %s\n", color.HighlightUserInput(services[0])) | ||
| return services[0], nil | ||
| } | ||
| selectedAppName, err := s.prompt.SelectOne(prompt, help, services) |
Contributor
There was a problem hiding this comment.
😂 wow that's an old name that stuck around for a while
efekarakus
approved these changes
Apr 21, 2021
thrau
pushed a commit
to localstack/copilot-cli-local
that referenced
this pull request
Dec 9, 2022
<!-- Provide summary of changes --> Currently our `svc/job delete` select candidates with local workspace files. However, this is not ideal since delete doesn't require to be in the workspace since we won't delete the local files. Also, this could hurt the idempotency since if the local files are removed, users cannot use `svc/job delete` to delete their workloads anymore. <!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" --> By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
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
4 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.
Currently our
svc/job deleteselect candidates with local workspace files. However, this is not ideal since delete doesn't require to be in the workspace since we won't delete the local files. Also, this could hurt the idempotency since if the local files are removed, users cannot usesvc/job deleteto delete their workloads anymore.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.