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

[sdk/python] Adds a default exception when dependency cycles are created #14597

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

kpitzen
Copy link
Contributor

@kpitzen kpitzen commented Nov 16, 2023

Description

Currently, when we detect that we've created a cycle in the dependency graph, we early-exit. This works well enough for simple cycles, but when early exiting is not sufficient (as when providing a resource output as an argument to another resource's inputs), we will still fail to resolve the full dependency graph. This PR introduces an exception-by-default, as any cycles represent an unsafe/invalid dependency graph and should be resolved manually. We also provide an escape hatch to fall back to current behavior, in case users would prefer to retain the ability to create unsafe dependency graphs (potentially introducing infinite hangs when resolving those graphs).

Fixes #13551

Checklist

  • I have run make tidy to update any new dependencies
  • I have run make lint to verify my code passes the lint check
    • I have formatted my code using gofumpt
  • I have added tests that prove my fix is effective or that my feature works
  • I have run make changelog and committed the changelog/pending/<file> documenting my change
  • Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Nov 16, 2023

Changelog

[uncommitted] (2023-11-17)

Bug Fixes

  • [sdk/python] Introduces RuntimeError when we detect a cycle upon adding dependencies to the graph. Additionally adds "PULUMI_ERROR_ON_DEPENDENCY_CYCLES" as a new environment variable to control this behavior. Set to False to return to the previous behavior, which could potentially re-introduce infinite hangs for some programs.
    #14597

@kpitzen kpitzen marked this pull request as ready for review November 16, 2023 21:33
@kpitzen
Copy link
Contributor Author

kpitzen commented Nov 16, 2023

Additional comment - what's the best way to document this new python-specific env var? Being a bit more explicit in the changelog? Not sure what the process looks like now.

@Frassle Frassle changed the title Adds a default exception to Python SDK when dependency cycles are cre… Adds a default exception to Python SDK when dependency cycles are created Nov 16, 2023
@Frassle
Copy link
Member

Frassle commented Nov 16, 2023

Additional comment - what's the best way to document this new python-specific env var? Being a bit more explicit in the changelog? Not sure what the process looks like now.

Firstly it should be prefixed with "PULUMI_", secondly yes probably changelog, but also you can add it to the sdk/env file even if none of the Go code currently uses it yet. Will then show up in pulumi env. I'd guess if we need to do similar fixes in other SDKs we'd then make sure we use the same envvar.

@kpitzen
Copy link
Contributor Author

kpitzen commented Nov 16, 2023

Additional comment - what's the best way to document this new python-specific env var? Being a bit more explicit in the changelog? Not sure what the process looks like now.

Firstly it should be prefixed with "PULUMI_", secondly yes probably changelog, but also you can add it to the sdk/env file even if none of the Go code currently uses it yet. Will then show up in pulumi env. I'd guess if we need to do similar fixes in other SDKs we'd then make sure we use the same envvar.

Got it - it is prefixed with PULUMI_ already, but good call on adding it to the go env file. I'll do that.

@kpitzen kpitzen force-pushed the KP/13551 branch 6 times, most recently from 5dcc4e1 to 95f4bdb Compare November 17, 2023 21:55
@kpitzen kpitzen changed the title Adds a default exception to Python SDK when dependency cycles are created [sdk/python] Adds a default exception when dependency cycles are created Nov 17, 2023
@kpitzen kpitzen added this pull request to the merge queue Nov 21, 2023
Merged via the queue into master with commit 7b21b5d Nov 21, 2023
44 checks passed
@kpitzen kpitzen deleted the KP/13551 branch November 21, 2023 17:00
github-merge-queue bot pushed a commit that referenced this pull request Nov 30, 2023
### Features

- [cli/config] Include config values from ESC in `pulumi config`
  [#14560](#14560)

- [cli/config] Add commands for managing stack environments
  [#14628](#14628)

- [cli/config] Add a command to create an ESC environment from stack
config
  [#14634](#14634)

- [sdk/go] add optional display name and tag fields to project templates
  [#14587](#14587)

- [cli/plugin] Load policy packs in parallel on startup to reduce
startup time
  [#14495](#14495)

- [sdkgen/{go,nodejs,python}] Resource methods with plain: true outputs
can now return plain values without an Output wrapper. In particular,
this feature enables resource methods to serve as explicit provider
factories by returning preconfigured explicit providers.
  [#13592](#13592)


### Bug Fixes

- [auto/go] Fix a datarace in cloning git repos.
  [#14643](#14643)

- [auto/go] Fixes event stream lag on windows runtime
  [#14659](#14659)

- [engine] Engine now correctly handles any resource name.
  [#14107](#14107)

- [engine] Fix a panic in cancellation.
  [#14612](#14612)

- [engine] Fix root directory passed to langauge plugins when starting
pulumi in a subfolder.
  [#14684](#14684)

- [sdkgen] Schemas now validate that 'urn' and 'id' are not used as
resource output properties.
  [#14637](#14637)

- [sdkgen] Fixes marshalling the "plain" flag from object or resource
properties
  [#14648](#14648)

- [programgen/nodejs] Fix generated readFile function so that it
includes the encoding and returns a string
  [#14633](#14633)

- [sdkgen/{dotnet,go,nodejs,python}] No longer writing out name and
project from alias definitions into SDKs, only type
  [#14625](#14625)

- [sdk/go] Fix optional handling on nested props
  [#14629](#14629)

- [sdkgen/go] Fixes plain and optional properties for generated types
for Go SDKs using generics
  [#14616](#14616)

- [sdkgen/go] Generate non-plain type variants for types used as inputs
inside unions
  [#14679](#14679)

- [sdk/python] Introduces RuntimeError when we detect a cycle upon
adding dependencies to the graph. Additionally adds
"PULUMI_ERROR_ON_DEPENDENCY_CYCLES" as a new environment variable to
control this behavior. Set to `False` to return to the previous
behavior, which could potentially re-introduce infinite hangs for some
programs.
  [#14597](#14597)
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.

ComponentResource hangs when a resource depends on its parent
3 participants