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

Fix attach to enable PULUMI_DEBUG_PROVIDERS #1716

Merged
merged 2 commits into from
Mar 5, 2024
Merged

Fix attach to enable PULUMI_DEBUG_PROVIDERS #1716

merged 2 commits into from
Mar 5, 2024

Conversation

t0yv0
Copy link
Member

@t0yv0 t0yv0 commented Feb 28, 2024

Due to a small omission PULUMI_DEBUG_PROVIDERS did not work before this change with providers utilizing Plugin Framework, which includes GCP and AWS. It now works.

"github.com/pulumi/providertest/pulumitest/opttest"
)

func TestAttach(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is currently failing due to the problem with YAML not supporting PULUMI_DEBUG_PROVIDERS. It should work once pulumi/pulumi#15526 is finalized and propagated to pulumi-yaml

@t0yv0
Copy link
Member Author

t0yv0 commented Feb 28, 2024

I can probably check this in with a test skip until we sort out the dependency, WDYT?

@VenelinMartinov
Copy link
Contributor

I'm happy with that - seems unlikely to affect any production code

Copy link

codecov bot commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 59.19%. Comparing base (d395661) to head (5058e7b).

Files Patch % Lines
pf/tfbridge/provider_attach.go 0.00% 7 Missing ⚠️
pf/internal/plugin/provider_server.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1716      +/-   ##
==========================================
- Coverage   59.74%   59.19%   -0.55%     
==========================================
  Files         300      308       +8     
  Lines       42025    42412     +387     
==========================================
- Hits        25109    25107       -2     
- Misses      15482    15871     +389     
  Partials     1434     1434              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@t0yv0
Copy link
Member Author

t0yv0 commented Feb 28, 2024

Error: C:\Users\runneradmin\go\pkg\mod\github.com\pulumi\providertest@v0.0.10\pulumitest\copy.go:69:40: undefined: syscall.Stat_t

What can this possibly be.

With these change providers based on Plugin Framework as well as muxed providers that utilize Plugin
Framework for a subset of their resources gain support for the PULUMI_DEBUG_PROVIDERS env var.

For example, this will now work:

    # Start an AWS provider process, allowing to attach a debugger
    $ pulumi-resource-aws
    12345

    # Instruct Pulumi to connect to the provider process instead of starting a new one
    PULUMI_DEBUG_PROVIDERS=aws:12345 pulumi up
@t0yv0 t0yv0 marked this pull request as ready for review March 5, 2024 14:12
@t0yv0 t0yv0 added this to the 0.101 milestone Mar 5, 2024
@t0yv0 t0yv0 merged commit d10e056 into master Mar 5, 2024
9 checks passed
@t0yv0 t0yv0 deleted the t0yv0/fix-attach branch March 5, 2024 14:28
iwahbe added a commit that referenced this pull request Sep 11, 2024
As part of #2148, this PR removes the duplicate implementation of
`pulumigrpc.ResourceProviderServer` copied over from pulumi/pulumi and then modified.

This is a post #2258 version of #2195, and is much less invasive.

Addressing previous edits in response to
#2195 (comment):

- #1716: This change accommodated for a previous edit (`ProviderWithContext`) and is no
  longer necessary. That said, the tests it introduced are still present (and continue to
  pass), so we are sure this commit did not break the functionality.
- #1683: Simply brings DiffConfig into line with pu/pu, which we get for free by not
  vendoring.
- #1047: This is still handled in `provider_server.go`, and is still under test.
- #1065: This has already been moved out of this area in #2258, so it is no longer
  relevant here.
iwahbe added a commit that referenced this pull request Sep 11, 2024
As part of #2148, this PR removes the duplicate implementation of
`pulumigrpc.ResourceProviderServer` copied over from pulumi/pulumi and then modified.

This is a post #2258 version of #2195, and is much less invasive.

Addressing previous edits in response to
#2195 (comment):

- #1716: This change accommodated for a previous edit (`ProviderWithContext`) and is no
  longer necessary. That said, the tests it introduced are still present (and continue to
  pass), so we are sure this commit did not break the functionality.
- #1683: Simply brings DiffConfig into line with pu/pu, which we get for free by not
  vendoring.
- #1047: This is still handled in `provider_server.go`, and is still under test.
- #1065: This has already been moved out of this area in #2258, so it is no longer
  relevant here.
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.

2 participants