search

pulumi / pulumi-terraform-bridge

A library allowing Terraform providers to be bridged into Pulumi.
Apache License 2.0
194 stars 43 forks source link

Normalize Read result for empty collections #2069

Closed t0yv0 closed 3 months ago

t0yv0 commented 3 months ago

This workaround is aimed to reduce dirty refresh incidence on diffs between empty and null collections that is benign in TF but projects worse to Pulumi.

codecov[bot] commented 3 months ago

Codecov Report

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

Project coverage is 61.20%. Comparing base (b012fa8) to head (437997b).

Files Patch % Lines
pkg/tfshim/sdk-v2/provider2.go 58.33% 9 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2069 +/- ## ========================================== - Coverage 61.21% 61.20% -0.01% ========================================== Files 337 337 Lines 45177 45200 +23 ========================================== + Hits 27654 27666 +12 - Misses 16003 16013 +10 - Partials 1520 1521 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

t0yv0 commented 3 months ago

At this point it's making TestAccBucketPy pass dirty refresh check in AWS. The change has to be scoped down to PlanResourceChange path because rolling it out generally in the bridge introduces more dirty refresh than we had before. This is pretty interesting, we might need to go super-surgical.

@VenelinMartinov suggested the correction may only be needed for maps, let me check lists and sets shortly.

t0yv0 commented 3 months ago

Guessing based on the investigation in https://github.com/pulumi/pulumi-terraform-bridge/pull/2063 that this needs to be scoped to non-computed collections. The best end to end test here would be to cover the idea that we get a clean refresh for these attributes.

t0yv0 commented 3 months ago

Closing in favor of #2073