pulumi / pulumi

Pulumi - Infrastructure as Code in any programming language 🚀
https://www.pulumi.com
Apache License 2.0
21.95k stars 1.13k forks source link

Suggestion: Move errors/diagnostic messages from "info" column elsewhere #1601

Open chrsmith opened 6 years ago

chrsmith commented 6 years ago

When a log message or error occurs during a pulumi update, it gets rendered on the "Info" column. This is difficult to read at best, and for messages that span multiple lines, gets truncated before it can be useful.

Example:


Do you want to perform this update? yes
Updating stack 'eliza-bot'
Performing changes:

     Type                                 Name                              Status                  Info
 +   pulumi:pulumi:Stack                  eliza-bot-eliza-bot               creating.
 +   ├─ twilio:rest:IncomingPhoneNumber   sms-handler                       creating.
 +   │  └─ aws-serverless:apigateway:API  sms-handler-api                   creating.
 +   │     └─ aws:apigateway:RestApi      sms-handler-api                   **creating failed**     1 error. error: Plan apply failed: creating urn:
 +   └─ aws:serverless:Function           sms-handler-apic9e56dfd           creating.
 +      ├─ aws:iam:Role                   sms-handler-apic9e56dfd           created
 +      ├─ aws:iam:RolePolicyAttachment   sms-handler-apic9e56dfd-32be53a2  created
 +      └─ aws:lambda:Function            sms-handler-apic9e56dfd           created

Would it be possible to render the full console.log messages or error messages below the "summary grid"? e.g. create a new diagnostics section on-demand? Something like:

...
 +      └─ aws:lambda:Function            sms-handler-apic9e56dfd           created
Diagnostics
Processing aws:apigateway:RestApi:
Plan apply failed: creating urn:pulumi:eliza-bot::eliza-bot::twilio:rest:IncomingPhoneNumber$aws-serverless:apigateway:API$aws:apigateway/restApi:RestApi::sms-handler-api: Error creating API Gateway: BadRequestException: Maximum number of APIs has been reached. Please contact AWS if you need additional APIs.
    status code: 400, request id: 9303bdb4-8151-11e8-8a34-7fdfd329772d

@CyrusNajmabadi WDYT?

ellismg commented 6 years ago

Don't we print the full set of diagnostics at the end of the update regardless?

CyrusNajmabadi commented 6 years ago

As @ellismg mentions, we print this all out at the end. Showing below while the operation is happening is doable, but something we need to be careful with. We don't want the error being so large it means you don't see progress.

One thing might just be to print out the first error diagnostic below, limited to no more then 4 lines.

CyrusNajmabadi commented 6 years ago

Moving out. No obvious 'good' solution is coming. A nd i think the experience now is actually ok. Note: you can also widen your screen to see more info here.

lukehoban commented 6 years ago

@pgavlin Suggested a "simple" option. On narrow terminals, put the "Info" and possibly "Name" columns into additional rows. Still keep a fixed number of rows per node in the tree - just allow it to be 2 or 3 instead of just 1. Pick this up front based on terminal width at time of launch.

Note that we are also working on improving the display of this same information in app.pulumi.com, and should consider printing the link to the update page before starting the update progress display so that users can go there to get a richer display of progress.

CyrusNajmabadi commented 6 years ago

This may be worth trying out. Though i'm somewhat skeptical this will provide enough marginal benefit to be worthwhile. For example, if the display is narrow, and we place the 'info' on hte line below, we're effectively betting on the useful information still having to be on the last printed line, close enough to the start to not be cut off.

If the necessary info is on a previous line, or is later in the same line, this will still be cut off.

Overall, i'm wary about dumping lots of work into a UI experience that is going to always be somewhat encumbered by the inherent limitations of text UI. I'd far prefer to just go simple and have the link to the web progress given up front. Then, if you see that there's an issue during the run, you can just go there, and we can more easily present complex live information thanks to the flexibility of an html front-end. With a normal UI framework, we can more easily do things like have the UI reflow naturally based on the client dimensions. We can easily have sections that can present rich detailed information, but also be collapsible for brevity. etc. etc.

joeduffy commented 6 years ago

This is related to https://github.com/pulumi/pulumi/issues/1851.

lukehoban commented 6 years ago

Moving to M20 along with #1851.

joeduffy commented 5 years ago

I have to admit, I'm not keen on changing our CLI display at the moment. I think it's "good enough" and we spent multiple milestones tweaking things to get to where we are.

@CyrusNajmabadi @lukehoban I propose we punt this down the line a bit longer, and focus on higher value work like progress on libraries, customer reported issues, and the like. Ok?

CyrusNajmabadi commented 5 years ago

That's fine with me. My headspace is not with the CLI right now. It's polish, whereas we have core critical functionality in other places we're trying to get done.

brandonkal commented 4 years ago

I also find it hard to read info as it is often cropped off and important feedback is lost if something is in a stuck state. I usually end up decreasing font size significantly when I wish to read the info column.

A keyboard shortcut that toggles on and off the first three rows, would go a long way to "zoom in" and read info messages/warnings.

Some other suggestions:

  1. Reduce spacing from + to Type column from 3 to 1 character
  2. Shorten type URNs for display. This column is often quite long, pushing everything to the side. For example:
    kubernetes:admissionregistration.k8s.io:ValidatingWebhookConfiguration
    k8s:ValidatingWebhookConfig
    kubernetes:rbac.authorization.k8s.io:ClusterRoleBinding
    k8s:CRB
  3. Truncate Type and Name columns
    kubernetes:admissionregistration.k8s.io:ValidatingWebhookConfiguration
    ...:ValidatingWebhookConfiguration
  4. **creating failed** is about 2.5x as long as created. Consider just failed in a color terminal.
  5. Consider placing info on its own line below status when there is a message.
dixler commented 2 years ago

Couldn't we print new diagnostic lines at the top of display and append new lines as log messages are output and then move the progress display to the bottom?

i.e.

Previewing update (dev)

View Live: https://app.pulumi.com/dixler/aws-python/dev/previews/16413561-38d4-4892-9a70-c28015217213

     Type                 Name            Plan       Info
 +   pulumi:pulumi:Stack  aws-python-dev  create     3 errors; 4 messages
 +   ├─ aws:s3:Bucket     bucket2         create     
 +   ├─ aws:s3:Bucket     bucket1         create     
 +   └─ aws:s3:Bucket     bucket3         create     

Diagnostics:
  pulumi:pulumi:Stack (aws-python-dev):
    log message 1
    log message 2
    <APPEND NEW LOGS HERE>

edit: I've looked into the display code and I think the cost to stream at the bottom would be cheap.