Commit Graph

19 Commits

Author SHA1 Message Date
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
Luke Hoban bb3aa1c529
Add `--suppress-progresss` flag to CLI ()
Adds support for suppressing the periodic "..." printing that can
disrupt normal output stream. This output is still deemed necessary to
include by default for CI systems that might otherwise cancel an update
that goes for a long time without printing anything. But we now have an
option to turn this off.

Notes:
1. We want to expose this via Automtation API, and may even want to
default to off via Automation API?

Fixes https://github.com/pulumi/pulumi/issues/14069.
Related https://github.com/pulumi/pulumi/issues/11139.
2024-02-05 11:48:10 +00:00
Fraser Waters 16d9f4c167
Enable perfsprint linter ()
<!--- 
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. -->

Prompted by a comment in another review:
https://github.com/pulumi/pulumi/pull/14654#discussion_r1419995945

This lints that we don't use `fmt.Errorf` when `errors.New` will
suffice, it also covers a load of other cases where `Sprintf` is
sub-optimal.

Most of these edits were made by running `perfsprint --fix`.

## 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
  - [x] 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. -->
2023-12-12 12:19:42 +00:00
Abhinav Gupta 7aa5b77a0c
all: Reformat with gofumpt
Per team discussion, switching to gofumpt.

[gofumpt][1] is an alternative, stricter alternative to gofmt.
It addresses other stylistic concerns that gofmt doesn't yet cover.

  [1]: https://github.com/mvdan/gofumpt

See the full list of [Added rules][2], but it includes:

- Dropping empty lines around function bodies
- Dropping unnecessary variable grouping when there's only one variable
- Ensuring an empty line between multi-line functions
- simplification (`-s` in gofmt) is always enabled
- Ensuring multi-line function signatures end with
  `) {` on a separate line.

  [2]: https://github.com/mvdan/gofumpt#Added-rules

gofumpt is stricter, but there's no lock-in.
All gofumpt output is valid gofmt output,
so if we decide we don't like it, it's easy to switch back
without any code changes.

gofumpt support is built into the tooling we use for development
so this won't change development workflows.

- golangci-lint includes a gofumpt check (enabled in this PR)
- gopls, the LSP for Go, includes a gofumpt option
  (see [installation instrutions][3])

  [3]: https://github.com/mvdan/gofumpt#installation

This change was generated by running:

```bash
gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error)
```

The following files were manually tweaked afterwards:

- pkg/cmd/pulumi/stack_change_secrets_provider.go:
  one of the lines overflowed and had comments in an inconvenient place
- pkg/cmd/pulumi/destroy.go:
  `var x T = y` where `T` wasn't necessary
- pkg/cmd/pulumi/policy_new.go:
  long line because of error message
- pkg/backend/snapshot_test.go:
  long line trying to assign three variables in the same assignment

I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 09:00:24 -08:00
Abhinav Gupta 3d64521cf2
unused: Remove unused functions and types
Several functions and types were reported as unused.

Most notably, jsonTemp and the method in jsonSpiller that uses it
were all unused.
2023-01-12 09:55:34 -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 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
Pat Gavlin 4d099ae343 [display] Factor out the low-level progress renderer.
The progress display is logically composed of two pieces:
- An event listener that transforms raw engine events into renderable
  data
- A renderer that renders that data on to the screen

The interface for the latter is relatively simple. It only really needs
to know the following:
- When an idle interval has elapsed
- When a row has changed
- When a system message (e.g. a line printed to stdout) has arrived
- When the update is done
- When the display is closed
- When to print a raw line

This interface is general enough to accommodate multiple renderers.
2022-10-31 07:59:14 -07:00
pulumi-bot 73a66f48ea [breaking] Changing the version of go.mod in sdk / pkg to be v3 2021-04-14 19:32:18 +01:00
CyrusNajmabadi 66bd3f4aa8
Breaking changes due to Feature 2.0 work
* Make `async:true` the default for `invoke` calls ()

* Switch away from native grpc impl. ()

* Remove usage of the 'deasync' library from @pulumi/pulumi. ()

* Only retry as long as we get unavailable back.  Anything else continues. ()

* Handle all errors for now. ()


* Do not assume --yes was present when using pulumi in non-interactive mode ()

* Upgrade all paths for sdk and pkg to v2

* Backport C# invoke classes and other recent gen changes ()

Adjust C# generation

* Replace IDeployment with a sealed class ()

Replace IDeployment with a sealed class

* .NET: default to args subtype rather than Args.Empty ()

* Adding system namespace for Dotnet code gen

This is required for using Obsolute attributes for deprecations

```
Iam/InstanceProfile.cs(142,10): error CS0246: The type or namespace name 'ObsoleteAttribute' could not be found (are you missing a using directive or an assembly reference?) [/Users/stack72/code/go/src/github.com/pulumi/pulumi-aws/sdk/dotnet/Pulumi.Aws.csproj]
Iam/InstanceProfile.cs(142,10): error CS0246: The type or namespace name 'Obsolete' could not be found (are you missing a using directive or an assembly reference?) [/Users/stack72/code/go/src/github.com/pulumi/pulumi-aws/sdk/dotnet/Pulumi.Aws.csproj]
```

* Fix the nullability of config type properties in C# codegen ()
2020-04-14 09:30:25 +01:00
evanboyle fccf301d14 move pkg/util/contract -> sdk/go/common/util/contract 2020-03-18 14:40:07 -07:00
James Nugent 8fed68f2b0 Depend on `ijc` fork of Gotty
This removes the need for a replace directive in every downstream `go.mod`,
however it does not protect against the case of a `go.mod` being added upstream
with a different import path in future. This seems unlikely given the cadence of
work upstream, however.
2019-07-25 15:37:23 -05:00
James Nugent edab10e9c8 Use Go Modules for dependency tracking
This commit switches from dep to Go 1.12 modules for tracking Pulumi
dependencies. Rather than _building_ using Go modules, we instead use the `go
mod vendor` command to populate a vendor tree in the same way as `dep ensure`
was previously doing.

In order to prevent checksum mismatches, it was necessary to also update CI to
use Go 1.12 instead of 1.11 - which also necessitated fixing some linting errors
which appeared with the upgraded golangci-lint for 1.12.
2019-04-10 08:37:51 +04:00
joeduffy b1f7cf7050 Fix a few lint warnings 2018-09-05 08:25:23 -07:00
joeduffy bf51d7594a Refactor display logic out of pkg/backend/filestate
This simply refactors all the display logic out of the
pkg/backend/filestate package. This helps to gear us up to better unify
this logic between the filestate and httpstate backends.

Furthermore, this really ought to be in its own non-backend,
CLI-specific package, but I'm taking one step at a time here.
2018-09-05 07:33:18 -07:00