Commit Graph

19 Commits

Author SHA1 Message Date
Thomas Gummerer fc10da33d9
Add display to the engine tests ()
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.

It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.

There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.

The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after https://github.com/pulumi/pulumi/pull/15607.

I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.

One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.

The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?

---------

Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Paul C. Roberts d864cce061
Decouple persist and display events ()
<!--- 
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->

# Description

Retry  with fix for the issue that required the revert in  

This removes a scenario where events could not be persisted to the cloud
because they were waiting on the same event being displayed

Instead of rendering the tree every time a row is updated, instead, this
renders when the display actually happens in the the `frame` call. The
renderer instead simply marks itself as dirty in the `rowUpdated`,
`tick`, `systemMessage` and `done` methods and relies on the frame being
redrawn on a 60Hz timer (the `done` method calls `frame` explicitly).
This makes the rowUpdated call exceedingly cheap (it simply marks the
treeRenderer as dirty) which allows the ProgressDisplay instance to
service the display events faster, which prevents it from blocking the
persist events.

This requires a minor refactor to ensure that the display object is
available in the frame method

Because the treeRenderer is calling back into the ProgressDisplay object
in a goroutine, the ProgressDisplay object needs to be thread safe, so a
read-write mutex is added to protect the `eventUrnToResourceRow` map.
The unused `urnToID` map was removed in passing.

## Impact

There are scenarios where the total time taken for an operation was
dominated by servicing the events.

This reduces the time for a complex (~2000 resources) `pulumi preview`
from 1m45s to 45s

For a `pulumi up` with `-v=11` on a the same stack, where all the
register resource spans were completing in 1h6m and the
postEngineEventBatch events were taking 3h45m, this PR removes the time
impact of reporting the events (greatly inflated by the high verbosity
setting) and the operation takes the anticipated 1h6m


<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->

Fixes  

This was happening because the renderer was being marked dirty once per
second in a tick event, which caused frame to redraw. There is a check
in the render method that `display.headerRow` is not nil that was
previously used to prevent rendering when no events had been added. This
check is now part of the `markDirty` logic

Some of the tests needed to be updated to make this work and have also
been refactored

## Checklist

- [X] I have run `make tidy` to update any new dependencies
- [X] I have run `make lint` to verify my code passes the lint check
  - [ ] I have formatted my code using `gofumpt`

<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [X] I have added tests that prove my fix is effective or that my
feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->

---------

Co-authored-by: Paul Roberts <proberts@pulumi.com>
2024-03-18 16:53:13 +00:00
Fraser Waters c465c02f29
Revert "Decouple persist and display events ()" ()
<!--- 
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->

This reverts commit ff7a7b4975.

Fixes https://github.com/pulumi/pulumi/issues/15668.
2024-03-15 20:37:31 +00:00
Paul C. Roberts ff7a7b4975
Decouple persist and display events ()
# Description

This removes a scenario where events could not be persisted to the cloud
because they were waiting on the same event being displayed

~This uses the same buffer size for both the display and persist
channels~ [Removed to make PR a single change]

The primary change, however, is to stop rendering the tree every time a
row is updated, instead, theis renders when the display actually happens
in the the `frame` call. The renderer instead simply marks itself as
dirty in the `rowUpdated`, `tick`, `systemMessage` and `done` methods
and relies on the frame being redrawn on a 60Hz timer (the `done` method
calls `frame` explicitly). This makes the rowUpdated call exceedingly
cheap (it simply marks the treeRenderer as dirty) which allows the
ProgressDisplay instance to service the display events faster, which
prevents it from blocking the persist events.

This requires a minor refactor to ensure that the display object is
available in the frame method

Because the treeRenderer is calling back into the ProgressDisplay object
in a goroutine, the ProgressDisplay object needs to be thread safe, so a
read-write mutex is added to protect the `eventUrnToResourceRow` map.
The unused `urnToID` map was removed in passing.

## Impact

There are scenarios where the total time taken for an operation was
dominated by servicing the events.

This reduces the time for a complex (~2000 resources) `pulumi preview`
from 1m45s to 45s

For a `pulumi up` with `-v=11` on a the same stack, where all the
register resource spans were completing in 1h6m and the
postEngineEventBatch events were taking 3h45m, this PR removes the time
impact of reporting the events (greatly inflated by the high verbosity
setting) and the operation takes the anticipated 1h6m

<!--- 
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->

<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->

Fixes # (issue)

## Checklist

- [X] I have run `make tidy` to update any new dependencies
- [X] I have run `make lint` to verify my code passes the lint check
  - [ ] I have formatted my code using `gofumpt`

<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my
feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->

---------

Co-authored-by: Paul Roberts <proberts@pulumi.com>
Co-authored-by: Thomas Gummerer <t.gummerer@gmail.com>
2024-03-04 20:22:26 +00:00
Paul C. Roberts fdd2d8eb39
Add vim shortcuts and Home and End Keys to terminal ()
<!--- 
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->

# Description

Updates the terminal support for the tree view in Pulumi CLI

This adds the vim shortcuts :
`j` &ndash; down
`k` &ndash; up
`Ctrl+F` &ndash; page down 
`Ctrl+B` &ndash; page up
`g` &ndash; home
`G` &ndash; end

Also adds Home and End key support

Fixes # (issue)

## Checklist

- [X] I have run `make tidy` to update any new dependencies
- [X] I have run `make lint` to verify my code passes the lint check
  - [ ] I have formatted my code using `gofumpt`

<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [X] I have added tests that prove my fix is effective or that my
feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [X] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->

Co-authored-by: Paul Roberts <proberts@pulumi.com>
2024-02-24 08:31:36 +00:00
Kyle Dixler a2dce880f2
Fixed panic in display on terminals with few rows and columns.
Users may resize their terminals during updates and this could cause the
bounds to go negative in the interactive update display. This commit
clamps the values to prevent panics.
2023-03-30 16:16:48 -07:00
Abhinav Gupta 13bcb56069
pkg: Dedupe "open in browser" library
We currently have two different "open in browser" libraries
imported into our system:

    github.com/pkg/browser
    github.com/skratchdot/open-golang/open

The latter hasn't been touched in nearly ten years.
Switch to pkg/browser and delete the unused dependency.
2023-03-10 10:57:33 -07:00
bors[bot] 5890e2e32c
Merge
12380: [cli] Add an "open in browser" shortcut r=pgavlin a=pgavlin

Add support for using ctrl+o to open the current update in the browser
for backends that support permalinks.

Co-authored-by: Pat Gavlin <pat@pulumi.com>
2023-03-10 00:16:48 +00:00
Pat Gavlin 2bad0a85e8 [cli] Add an "open in browser" keybinding
Add support for using Ctrl+O to open the current update in the browser
for backends that support permalinks.

The keybinding is advertised in the interactive display as part of the
message that displays the permalink:

```
Previewing update (dev)

View in Browser (Ctrl+O): https://<some-url>

     Type                          Name      Plan
 +   pulumi:pulumi:Stack           vpc-dev   create
 +   ├─ aws:ec2:Vpc                vpc       create
 +   ├─ aws:ec2:SecurityGroup      secgroup  create
 +   ├─ aws:ec2:SecurityGroupRule  rule-2    create
 +   ├─ aws:ec2:SecurityGroupRule  rule-0    create
 +   ├─ aws:ec2:SecurityGroupRule  rule-1    create
 +   └─ aws:ec2:SecurityGroupRule  rule-3    create
```

In order to maintain backwards compatibility with older versions of the
Automation API, the message is not changed for non-interactive
scenarios.
2023-03-09 15:52:13 -08:00
Abhinav Gupta acaf79bc10
all: Drop //nolint:goconst
Drops the nolint:goconst directive
now that the goconst linter is no longer enabled.
2023-03-09 11:15:21 -08:00
Pat Gavlin 5c77ce69fc [cli] Add autoscrolling and support page-{down,up}
- If the display is scrolled to the bottom, autoscroll it as new lines are
  added to the output.

- Add support for scrolling the interactive display using page-down and
  page-up. These keys scroll the display N lines at a time, where N is the
  height of the terminal.
2023-03-07 12:16:02 -08:00
Abhinav Gupta 0bff0b8716 sdk/go: Remove 'nolint' directives from package docs
Go treats comments that match the following regex as directives.

    //[a-z0-9]+:[a-z0-9]

Comments that are directives don't show in an entity's documentation.
5a550b6951 (diff-f56160fd9fcea272966a8a1d692ad9f49206fdd8dbcbfe384865a98cd9bc2749R165)

Our code has `//nolint` directives that now show in the API Reference.
This is because these directives are in one of the following forms,
which don't get this special treatment.

    // nolint:foo
    //nolint: foo

This change fixes all such directives found by the regex:
`// nolint|//nolint: `.
See bottom of commit for command used for the fix.

Verification:
Here's the output of `go doc` on some entities
before and after this change.

Before
```
% go doc github.com/pulumi/pulumi/sdk/v3/go/pulumi | head -n8
package pulumi // import "github.com/pulumi/pulumi/sdk/v3/go/pulumi"

nolint: lll, interfacer

nolint: lll, interfacer

const EnvOrganization = "PULUMI_ORGANIZATION" ...
var ErrPlugins = errors.New("pulumi: plugins requested")
```

After
```
% go doc github.com/pulumi/pulumi/sdk/v3/go/pulumi | head -n8
package pulumi // import "github.com/pulumi/pulumi/sdk/v3/go/pulumi"

const EnvOrganization = "PULUMI_ORGANIZATION" ...
var ErrPlugins = errors.New("pulumi: plugins requested")
func BoolRef(v bool) *bool
func Float64Ref(v float64) *float64
func IntRef(v int) *int
func IsSecret(o Output) bool
```

Before
```
% go doc github.com/pulumi/pulumi/sdk/v3/go/pulumi URN_
package pulumi // import "github.com/pulumi/pulumi/sdk/v3/go/pulumi"

func URN_(o string) ResourceOption
    URN_ is an optional URN of a previously-registered resource of this type to
    read from the engine. nolint: revive
```

After:
```
% go doc github.com/pulumi/pulumi/sdk/v3/go/pulumi URN_
package pulumi // import "github.com/pulumi/pulumi/sdk/v3/go/pulumi"

func URN_(o string) ResourceOption
    URN_ is an optional URN of a previously-registered resource of this type to
    read from the engine.
```

Note that golangci-lint offers a 'nolintlint'  linter
that finds such miuses of nolint,
but it also finds other issues so I've deferred that to a follow up PR.

Resolves 

Related: https://github.com/golangci/golangci-lint/issues/892

[git-generate]
FILES=$(mktemp)
rg -l '// nolint|//nolint: ' |
  tee "$FILES" |
  xargs perl -p -i -e '
    s|// nolint|//nolint|g;
    s|//nolint: |//nolint:|g;
  '
rg '.go$' < "$FILES" | xargs gofmt -w -s
2023-01-06 09:06:47 -08:00
Pat Gavlin f92cb3d5d0 [display] Hide the cursor in interactive mode.
Hiding the cursor should eliminate the last of the redraw artifacts.
2022-12-20 10:56:59 -08:00
Pat Gavlin 4fcf040ade [display] Overwrite terminal contents
Instead of clearing the contents of the display with each frame,
overwrite its contents by clearing to the end of each line prior to a
newline. This should eliminate flickering in the interactive display.

This also eliminates a couple of off-by-one errors that were hampering
usability.
2022-12-20 10:27:31 -08:00
Pat Gavlin 3aecebe5cb [color] Use graphemes to measure strings.
The number of Unicode code points in a string is not the same as the
number of user-visible characters (graphemes). When measuring colorized
strings, we want the latter rather than the former. Notably, these
changes fix some issues where the interactive display cut off before the
right edge of the terminal.
2022-11-09 08:23:00 -08:00
Pat Gavlin a20ddffde5 [cli] Abstract out terminal interactions
Replace direct interaction with the terminal with an abstraction. This
abstraction is tightly constrained to the capabilities needed for the
CLI's display. Using this abstraction allows for straightforward testing
of the interactive renderers.
2022-11-08 17:13:56 -08:00
Pat Gavlin 0e8d3e4577 write a status line for scrollable content 2022-11-08 08:56:01 -08:00
Pat Gavlin ecb88dae14 fall back to the message renderer on Windows 2022-11-07 22:02:44 -08:00
Pat Gavlin 27fb276f64 [cli] Reimplement the interactive renderer
The display pipleline looks like this:

       ╭──────╮
       │Engine│
       ╰──────╯
          ⬇ engine events
   ╭────────────────╮
   │Progress Display│
   ╰────────────────╯
          ⬇ display events: ticks, resource updates, system messages
   ╭─────────────────╮
   │Progress Renderer│
   ╰─────────────────╯
          ⬇ text
      ╭────────╮
      │Terminal│
      ╰────────╯

The existing implementation of the interactive Progress Renderer is broken
into two parts, the display renderer and the message renderer. The display
renderer converts display events into progress messages, each of which
generally represents a single line of text at a particular position in
the output. The message renderer converts progress messages into screen
updates by identifying whether or not the contents of a particular
message have changed and if so, re-rendering its output line. In
somewhat greater detail:

   ╭────────────────╮
   │Display Renderer│
   ╰────────────────╯
          ⬇ convert resource rows into a tree table
          ⬇ convert the tree table and system messages into lines
          ⬇ convert each line into a progress message with an index
   ╭────────────────╮
   │Message Renderer│
   ╰────────────────╯
          ⬇ if the line identified in a progress message has changed,
          ⬇ go to that line on the terminal, clear it, and update it
      ╭────────╮
      │Terminal│
      ╰────────╯

This separation of concerns is unnecessary and makes it difficult to
understand where and when the terminal is updated. This approach also
makes it somewhat challenging to change the way in which the display
interacts with the terminal, as both the display renderer and the
message renderer need to e.g. understand terminal dimensions, movement,
etc.

These changes reimplement the interactive Progress Renderer using a
frame-oriented approach. The display is updated at 60 frame per second.
If nothing has happened to invalidate the display's contents (i.e. no
changes to the terminal geometry or the displayable contents have occurred),
then the frame is not redrawn. Otherwise, the contents of the display
are re-rendered and redrawn.

An advantage of this approach is that it made it relatively simple to
fix a long-standing issue with the interactive display: when the number
of rows in the output exceed the height of the terminal, the new
renderer clamps the output and allows the user to scroll the tree table
using the up and down arrow keys.
2022-11-07 22:02:44 -08:00