Change 'policy new' to use the language host to install deps #10797
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changelog[uncommitted] (2022-09-20)Features
|
iwahbe
approved these changes
Sep 20, 2022
|
bors merge |
bors bot
added a commit
that referenced
this pull request
Sep 20, 2022
10797: Change 'policy new' to use the language host to install deps r=Frassle a=Frassle <!--- 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. --> In my efforts to continue removing all language specific code from the engine. This looked like it would be a really simply change given that we'd already fixed installed dependencies for "pulumi new" a while back. Turned out a little awkward because we assume the presence of a program project when making a plugin context. I've worked around that for now, copying the needed fields from the policy project to a fake program project but this is definitely a point to loop back to and cleanup. This also needed similar logic to "pulumi new" to handle the "python wants to set venv option" problem. That's another point to loop back on to see if we can improve it. ## Checklist <!--- 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. --> - [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 Service, 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 Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: Fraser Waters <fraser@pulumi.com>
|
Build failed: |
|
bors retry |
|
Build succeeded: |
bors bot
added a commit
that referenced
this pull request
Dec 13, 2022
11456: Implement YAML roundtripping, comment preserving edits r=AaronFriel a=AaronFriel By caching a copy of the raw bytes of the original loaded document - project, policy project, or stack - on save we: * Unmarshal the original bytes to a YAML AST * Recursively edit nodes of the document to match the values of the value we're saving, preserving trivia * Marshal the modified AST to []byte Closes #423, as we don't support comments in JSON presently (not using "JSON5") Closes #5235 Simplifies work done in #10797 and #10437 which used the previous `yamlutil` package to edit AST nodes directly. # Automation API Integration with automation API requires that operations either perform a "read-modify-write" in a single operation, or use a side channel to allow subsequent "read" (select stack?) and "write" operations to keep state of the original file. However I don't expect this to be needed. We don't have a reason to believe automation API users maintain comments in particular stack files, and doing so is already unsupported in automation scenarios. The Pulumi Service does not persist raw config files, thus `pulumi config refresh` and `CreateOrSelectStack` create config files without comments. n.b.: If you test this, due to the new behavior of this PR, `pulumi config refresh` will persist comments if there is an existing config file. I would expect automation API using local file workspaces to behave similarly. When using these tools to create a config file from the last deployment's definition, the file is created restored without comments. # Tradeoffs I believe that this closes #423 in spirit, but doesn't strictly implement round-tripping. There are a couple reasons for this. ## Marshaller interface and round-tripping simplification decisions We support unmarshaling "simplified" YAML, including allowing: ```yaml runtime: yaml # this unmarshals to a struct, not a scalar value config: someKey: "someScalar" # likewise ``` These simplifications make it much more difficult, the `Marshal` operation would _also_ need context of decisions made when unmarshaling to ensure a bijection. Otherwise we may save one of these as the other: ```yaml runtime: name: yaml ``` ```yaml runtime: yaml ``` This could be considered a feature, as it enforces a style guide - the decisions implemented in our custom `Marshaller` implementations become the style enforced on save. That said, I believe the new `Edit` API while not round-trippable, does reach a fixed point after one save through the new API. Once saved with this API, subsequent loads & saves without changes are idempotent. ## Indentation The YAML library uses a global indentation when marshaling values to YAML, as opposed to indentation being a property of AST nodes. This means we may lose other indentation. Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
bors bot
added a commit
that referenced
this pull request
Dec 13, 2022
11456: Implement YAML roundtripping, comment preserving edits r=AaronFriel a=AaronFriel By caching a copy of the raw bytes of the original loaded document - project, policy project, or stack - on save we: * Unmarshal the original bytes to a YAML AST * Recursively edit nodes of the document to match the values of the value we're saving, preserving trivia * Marshal the modified AST to []byte Closes #423, as we don't support comments in JSON presently (not using "JSON5") Closes #5235 Simplifies work done in #10797 and #10437 which used the previous `yamlutil` package to edit AST nodes directly. # Automation API Integration with automation API requires that operations either perform a "read-modify-write" in a single operation, or use a side channel to allow subsequent "read" (select stack?) and "write" operations to keep state of the original file. However I don't expect this to be needed. We don't have a reason to believe automation API users maintain comments in particular stack files, and doing so is already unsupported in automation scenarios. The Pulumi Service does not persist raw config files, thus `pulumi config refresh` and `CreateOrSelectStack` create config files without comments. n.b.: If you test this, due to the new behavior of this PR, `pulumi config refresh` will persist comments if there is an existing config file. I would expect automation API using local file workspaces to behave similarly. When using these tools to create a config file from the last deployment's definition, the file is created restored without comments. # Tradeoffs I believe that this closes #423 in spirit, but doesn't strictly implement round-tripping. There are a couple reasons for this. ## Marshaller interface and round-tripping simplification decisions We support unmarshaling "simplified" YAML, including allowing: ```yaml runtime: yaml # this unmarshals to a struct, not a scalar value config: someKey: "someScalar" # likewise ``` These simplifications make it much more difficult, the `Marshal` operation would _also_ need context of decisions made when unmarshaling to ensure a bijection. Otherwise we may save one of these as the other: ```yaml runtime: name: yaml ``` ```yaml runtime: yaml ``` This could be considered a feature, as it enforces a style guide - the decisions implemented in our custom `Marshaller` implementations become the style enforced on save. That said, I believe the new `Edit` API while not round-trippable, does reach a fixed point after one save through the new API. Once saved with this API, subsequent loads & saves without changes are idempotent. ## Indentation The YAML library uses a global indentation when marshaling values to YAML, as opposed to indentation being a property of AST nodes. This means we may lose other indentation. Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In my efforts to continue removing all language specific code from the engine. This looked like it would be a really simply change given that we'd already fixed installed dependencies for "pulumi new" a while back. Turned out a little awkward because we assume the presence of a program project when making a plugin context.
I've worked around that for now, copying the needed fields from the policy project to a fake program project but this is definitely a point to loop back to and cleanup.
This also needed similar logic to "pulumi new" to handle the "python wants to set venv option" problem. That's another point to loop back on to see if we can improve it.
Checklist
make changelogand committed thechangelog/pending/<file>documenting my change