add-to-ECS: select profile, region, and cluster #175

mgritter · 2022-11-08T05:07:01Z

This covers:

asking for a profile name
selecting a region from a list
listing the clusters in a region

This covers: * asking for a profile name * selecting a region from a list * listing the clusters in a region

mgritter · 2022-11-08T05:08:29Z

Example interaction:

mark@ubuntu:~/akita-cli$ bin/akita ecs  
? Which of your AWS profiles should Akita use to configure ECS? mgritter
[INFO] Successfully loaded AWS credentials.
? In which AWS region is your ECS cluster? us-west-2
[ERROR] Could not find any ECS clusters in this region. Please select a different one or hit Ctrl+C to exit.
? In which AWS region is your ECS cluster?  [Use arrows to move, type to filter, ? for more help]
> default
  find all clusters
  af-south-1
  ap-east-1
  ap-northeast-1
  ap-northeast-2
  ap-northeast-3
[INFO] Interrupted!

mgritter · 2022-11-08T05:11:42Z

cmd/internal/ecs/add.go

+	if awsRegionFlag != "" {
+		wf.awsRegion = awsRegionFlag
+		err = wf.createSession()


I have set this up so that if command-line arguments are provided, we do not prompt for that step at all.

The intent is that specifying all the command-line flags should be the same whether or not the program is running interactively. However, an alternative is that in interactive mode we could use the flags as the default and ask for confirmation. I'm still undecided, so feedback about when we ask for confirmation would be welcome: only at the end? At each step? Not at all?

How about if we waited until the end to confirm, but provided a command-line flag to skip this final confirmation?

liujed

I'll look at this in more detail later, but here are a couple of quick comments.

go.mod

cmd/internal/ecs/add.go

mgritter · 2022-11-08T23:12:17Z

Converting to draft until sdk v2 conversion is complete.

liujed

A few more comments. I'll do the rest of the review when the change to SDK v2 lands.

cmd/internal/ecs/add.go

Use DescribeRegions to populate region list, but continue fallback of having a list available; this API requires AIM permissions! Check permissions on operations and attempt to report them in a friendly fashion.

cmd/internal/ecs/add.go

liujed · 2022-11-11T09:14:30Z

cmd/internal/ecs/add.go

+				if name == "" {
+					return ""
+				}


When does this happen?

The cluster may not have a name.

cmd/internal/ecs/add.go

cmd/internal/ecs/aws_api.go

cmd/internal/ecs/ecs.go

liujed · 2022-11-11T09:58:00Z

GitHub appears confused.

Co-authored-by: Jed Liu <[email protected]>

Fix module import. Check for absence of any ECS clusters. UnrecognizedClientException handling.

First checkpoint on add-to-ECS command.

750113d

This covers: * asking for a profile name * selecting a region from a list * listing the clusters in a region

mgritter requested a review from a team November 8, 2022 05:07

mgritter commented Nov 8, 2022

View reviewed changes

liujed reviewed Nov 8, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

cmd/internal/ecs/add.go Outdated Show resolved Hide resolved

mgritter marked this pull request as draft November 8, 2022 23:11

liujed reviewed Nov 9, 2022

View reviewed changes

cmd/internal/ecs/add.go Outdated Show resolved Hide resolved

cmd/internal/ecs/add.go Outdated Show resolved Hide resolved

cmd/internal/ecs/add.go Outdated Show resolved Hide resolved

Update to V2 API.

b17df10

Use DescribeRegions to populate region list, but continue fallback of having a list available; this API requires AIM permissions! Check permissions on operations and attempt to report them in a friendly fashion.

mgritter changed the title ~~First checkpoint on add-to-ECS command.~~ add-to-ECS: select profile, region, and cluster Nov 11, 2022

Review feedback

536e911

mgritter marked this pull request as ready for review November 11, 2022 01:03

mgritter requested review from liujed and a team November 11, 2022 01:03

liujed approved these changes Nov 11, 2022

View reviewed changes

mgritter and others added 9 commits November 11, 2022 16:44

Update cmd/internal/ecs/add.go

0b65b22

Co-authored-by: Jed Liu <[email protected]>

Update cmd/internal/ecs/add.go

0f0ef6e

Co-authored-by: Jed Liu <[email protected]>

Update cmd/internal/ecs/add.go

4affe36

Co-authored-by: Jed Liu <[email protected]>

Update cmd/internal/ecs/aws_api.go

7ba04e2

Co-authored-by: Jed Liu <[email protected]>

Update cmd/internal/ecs/aws_api.go

f3588da

Co-authored-by: Jed Liu <[email protected]>

Update cmd/internal/ecs/ecs.go

17c91cc

Co-authored-by: Jed Liu <[email protected]>

Update cmd/internal/ecs/ecs.go

a2e9758

Co-authored-by: Jed Liu <[email protected]>

Update cmd/internal/ecs/ecs.go

efdc46f

Co-authored-by: Jed Liu <[email protected]>

Fix help message.

15c83dd

Fix module import. Check for absence of any ECS clusters. UnrecognizedClientException handling.

mgritter merged commit 84168a6 into main Nov 11, 2022

mgritter deleted the mgritter/ecs_command_and_profile_selection branch November 11, 2022 23:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add-to-ECS: select profile, region, and cluster #175

add-to-ECS: select profile, region, and cluster #175

mgritter commented Nov 8, 2022

mgritter commented Nov 8, 2022

mgritter Nov 8, 2022

liujed Nov 8, 2022

liujed left a comment

mgritter commented Nov 8, 2022

liujed left a comment

liujed Nov 11, 2022

mgritter Nov 11, 2022

liujed commented Nov 11, 2022

add-to-ECS: select profile, region, and cluster #175

add-to-ECS: select profile, region, and cluster #175

Conversation

mgritter commented Nov 8, 2022

mgritter commented Nov 8, 2022

mgritter Nov 8, 2022

Choose a reason for hiding this comment

liujed Nov 8, 2022

Choose a reason for hiding this comment

liujed left a comment

Choose a reason for hiding this comment

mgritter commented Nov 8, 2022

liujed left a comment

Choose a reason for hiding this comment

liujed Nov 11, 2022

Choose a reason for hiding this comment

mgritter Nov 11, 2022

Choose a reason for hiding this comment

liujed commented Nov 11, 2022