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

feat(cli): add app show support for backend app #876

Merged
merged 4 commits into from
Apr 28, 2020

Conversation

iamhopaul123
Copy link
Contributor

@iamhopaul123 iamhopaul123 commented Apr 28, 2020

With this change app show now should be able to show running backend applications. However, we cannot test this feature manually until app deploy supports deploying backend app.

Ideally the output for app show for backend app should be:

About

  Project           my-project
  Name              my-app
  Type              Backend App

Configurations

  Environment       Tasks               CPU (vCPU)          Memory (MiB)        Port
  test              1                   0.25                512                 5000
  prod              2                   0.5                 1024                5000

Service Discovery:

  test,prod         my-app.my-project.local:5000

Variables

  Name              Environment         Value

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@kohidave
Copy link
Contributor

Can we remove the http from the service discovery endpoint?

@efekarakus
Copy link
Contributor

However, we cannot test this feature manually until app deploy supports deploying backend app.

I'll pull your branch and test it with my WIP app deploy branch and report back :D

@efekarakus
Copy link
Contributor

Your command works!

$ ecs-preview app show
? Which application of phonetool would you like to show? rindex
About

  Project           phonetool
  Name              rindex
  Type              Backend App

Configurations

  Environment       Tasks               CPU (vCPU)          Memory (MiB)        Port
  test              1                   0.25                512                 80

Service Discovery

  Environment       Namespace
  test              http://rindex.phonetool.local:80

Variables

  Name                        Environment         Value
  ECS_APP_DISCOVERY_ENDPOINT  test                phonetool.local
  ECS_CLI_APP_NAME            test                rindex
  ECS_CLI_ENVIRONMENT_NAME    test                test
  ECS_CLI_LB_DNS              test                phone-Publi-1B334QZ3UBM1Y-1538761958.us-west-2.elb.amazonaws.com
  ECS_CLI_PROJECT_NAME        test                phonetool

@iamhopaul123
Copy link
Contributor Author

Your command works!

$ ecs-preview app show
? Which application of phonetool would you like to show? rindex
About

  Project           phonetool
  Name              rindex
  Type              Backend App

Configurations

  Environment       Tasks               CPU (vCPU)          Memory (MiB)        Port
  test              1                   0.25                512                 80

Service Discovery

  Environment       Namespace
  test              http://rindex.phonetool.local:80

Variables

  Name                        Environment         Value
  ECS_APP_DISCOVERY_ENDPOINT  test                phonetool.local
  ECS_CLI_APP_NAME            test                rindex
  ECS_CLI_ENVIRONMENT_NAME    test                test
  ECS_CLI_LB_DNS              test                phone-Publi-1B334QZ3UBM1Y-1538761958.us-west-2.elb.amazonaws.com
  ECS_CLI_PROJECT_NAME        test                phonetool

YAY!

var d *describe.WebAppDescriber
var err error
}
opts.initDescriber = func(enableResources bool) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

As we add more patterns we might want this to be a describer provider or something that can look up the right functions in a map or something like that - so we don't end up with one giant function. We needn't do it now, but just a note.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. That's a good idea!

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