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

chore: log cmd to create default cluster on task run error #2035

Merged
merged 3 commits into from
Mar 11, 2021

Conversation

ota42y
Copy link
Contributor

@ota42y ota42y commented Mar 10, 2021

When we don't set --env flags, Copilot run task on default cluster.
In the old description, it is difficult to understand what happens if we don't set --default flag.

Copilot allows to deploy the service to another region(environment) without changing profile's region, but task run without --env use default region only (perhaps) so I added description about region.
I think it is better to add --region option in run task command but there is a workaround so I can't say it should be done 🤔
Therefore, I added only the explanation first.

@ota42y ota42y requested a review from a team as a code owner March 10, 2021 07:09
@ota42y ota42y requested a review from efekarakus March 10, 2021 07:09
Copy link
Contributor

@efekarakus efekarakus left a comment

Choose a reason for hiding this comment

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

Thank you so much for improving the documentation! What do you think of the suggestion below?

@ota42y
Copy link
Contributor Author

ota42y commented Mar 11, 2021

I have completed the fix!

@@ -16,8 +16,7 @@ Generally, the steps involved in task run are:
!!!info
1. Tasks with the same group name share the same set of resources, including the CloudFormation stack, ECR repository, CloudWatch log group and task definition.
2. If the tasks are deployed to a Copilot environment (i.e. by specifying `--env`), only public subnets that are created by that environment will be used.
3. The `--env` flag only works with environments created with v0.3.0 of Copilot or later. Customers using environments created with v0.2.0 or earlier can update their environment manager role with [this](https://github.com/aws/copilot-cli/blob/mainline/templates/environment/cf/environment-manager-role.yml) policy.
4. If you are using the `--default` flag and get an error saying there's no default cluster, run `aws ecs create-cluster` and then re-run the Copilot command.
3. If you are using the `--default` flag and get an error saying there's no default cluster, run `aws ecs create-cluster` and then re-run the Copilot command.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also no longer needed :) since we moved it to the code

@efekarakus efekarakus changed the title fix(doc): task run option description to more clarified chore: log cmd to create default cluster on task run error Mar 11, 2021
Copy link
Contributor

@iamhopaul123 iamhopaul123 left a comment

Choose a reason for hiding this comment

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

yay!! Thank you!

@mergify mergify bot merged commit b86e7b5 into aws:mainline Mar 11, 2021
@ota42y ota42y deleted the patch-1 branch March 12, 2021 14:14
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
When we don't set `--env` flags, [Copilot run task on default cluster](https://github.com/aws/copilot-cli/blob/mainline/internal/pkg/cli/task_run.go#L197).
In the old description, it is difficult to understand what happens if we don't set `--default` flag.

Copilot allows to deploy the service to another region(environment) without changing profile's region, but task run without `--env` use default region only (perhaps) so I added description about region.
I think it is better to add `--region` option in `run task` command but there is a workaround so I can't say it should be done 🤔 
Therefore, I added only the explanation first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants