-
Notifications
You must be signed in to change notification settings - Fork 162
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
Remove remove-ephemeral-resources.patch #5339
Conversation
a4343d5
to
e21aa4e
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. Maintainer note: consult the runbook for dealing with any breaking changes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5339 +/- ##
==========================================
- Coverage 24.36% 24.31% -0.05%
==========================================
Files 364 364
Lines 145472 145577 +105
==========================================
- Hits 35448 35401 -47
- Misses 109925 110077 +152
Partials 99 99 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
func(propertyVisitInfo tfbridge.PropertyVisitInfo) (tfbridge.PropertyVisitResult, error) { | ||
schemaPath := propertyVisitInfo.SchemaPath() | ||
if key, ok := schemaPath[len(schemaPath)-1].(walk.GetAttrStep); ok { | ||
if strings.Contains(key.Name, "_wo_") || strings.HasSuffix(key.Name, "_wo") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this, this seems reasonable. If we are worried that this will over-eagerly skip a property that does not need skipping, we can add a file with a checklist of removed properties, say (resource, SchemaPath) pairs and force PRs to change that, perhaps in a test.
@@ -1,1002 +0,0 @@ | |||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | |||
From: guineveresaenger <guinevere@pulumi.com> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM.
This PR has been shipped in release v6.73.0. |
This pull request combines new bridge functionality and an attribute traversal to automate omitting any of the kind of write-only and write-only-adjacent resources that a patch addressed previously.
While there's a bit of metadata tracking cruft due to the attribute traversal wanting build-time information, the schema itself has no changes.
Fixes #5231.