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

Add an ignoreChanges resource option #2657

Merged
merged 5 commits into from
Apr 22, 2019
Merged

Conversation

lukehoban
Copy link
Member

@lukehoban lukehoban commented Apr 19, 2019

Fixes #2277.

Adds a new ignoreChanges resource option that allows specifying a list of property names whose values will be ignored during updates. The property values will be used for Create, but will be ignored for purposes of updates, and as a result also cannot trigger replacements.

This is a feature of the Pulumi engine, not of the resource providers, so no new logic is needed in providers to support this feature. Instead, the engine simply replaces the values of input properties in the goal state with old inputs for properties marked as ignoreChanges.

Currently, only top level properties may be specified in ignoreChanges. In the future, this could be extended to support paths to nested properties (including into array elements) with a JSONPath/JMESPath syntax.

Fixes #2277.

Adds a new ignoreChanges resource option that allows specifying a list of property names whose values will be ignored during updates.  The property values will be used for Create, but will be ignored for purposes of updates, and as a result also cannot trigger replacements.

This is a feature of the Pulumi engine, not of the resource providers, so no new logic is needed in providers to support this feature.  Instead, the engine simply replaces the values of input properties in the goal state with old inputs for properties marked as `ignoreChanges`.

Currently, only top level properties may be specified in `ignoreChanges`.  In the future, this could be extended to support paths to nested properties (including into array elements) with a JSONPath/JMESPath syntax.
/**
* Ignore changes to any of the specified properties.
*/
ignoreChanges?: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that you added this to ResourceOptions instead of CustomResourceOptions. What does ignoreChanges mean on component resource, since there are no CRUD operations associated with components?

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically, the feature does apply to both Component and Custom resources, so I think this is technically correct. That said, I'm not sure practically what impact it can have to ignore changes on a component (though in the UI and checkpoint, this will have the effect of ignoring input property changes). But I think that's more about some of the current oddness around input/output properties on components generally. I think for orthogonality purposes, it does make sense to keep this as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@swgillespie
Copy link
Contributor

Note: Python/Go support will be added soon - will not merge prior to adding these.

Can we open an issue for Python? The linked issue will be closed as soon as this PR merges.

@@ -177,6 +177,7 @@ export function registerResource(res: Resource, t: string, name: string, custom:
req.setProvider(resop.providerRef);
req.setDependenciesList(Array.from(resop.allDirectDependencyURNs));
req.setDeletebeforereplace((<any>opts).deleteBeforeReplace || false);
req.setIgnorechangesList(opts.ignoreChanges || []);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test that exercises this code path in the language host?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Also add SDK test coverage for JavaScript ignoreChanges.
@lukehoban
Copy link
Member Author

Can we open an issue for Python? The linked issue will be closed as soon as this PR merges.

I've added the Python support to this PR.

@@ -87,6 +92,7 @@ def __init__(self,
self.provider = provider
self.providers = providers
self.delete_before_replace = delete_before_replace
self.ignore_changes = ignore_changes
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a :param directive for this new parameter after line 87? This is what Sphinx picks up when it generates documentation.

@lukehoban lukehoban merged commit 0550f71 into master Apr 22, 2019
@pulumi-bot pulumi-bot deleted the lukehoban/ignoreChanges branch April 22, 2019 20:54
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.

Allow changes to property values to be ignored
3 participants