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

Implement purge_behavior parameter #60

Merged
merged 6 commits into from
Dec 27, 2020

Conversation

reidmv
Copy link
Contributor

@reidmv reidmv commented Nov 4, 2020

This allows for specifying classes or data keys that should be present or have defined values, without removing or modifying other data already present in the node group.

This allows for specifying classes or data keys that should be present
or have defined values, without removing or modifying other data already
present in the node group.
@reidmv
Copy link
Contributor Author

reidmv commented Nov 6, 2020

@WhatsARanjit care to review?

@reidmv
Copy link
Contributor Author

reidmv commented Dec 21, 2020

@WhatsARanjit does anyone else have merge access to this repo? If not, would you be open to designating a few people?

@WhatsARanjit
Copy link
Contributor

@reidmv Sorry for being absent. This is fine with me, but can you whip up some corresponding tests for purging in different scenarios? Thanks.

@reidmv
Copy link
Contributor Author

reidmv commented Dec 21, 2020

Sure, I'll take a stab at some basic tests. Will push an update when I have them.

@reidmv
Copy link
Contributor Author

reidmv commented Dec 21, 2020

@WhatsARanjit a base set of tests implemented and pushed to PR. Waiting for Travis to complete; had to change the Gemfile to get a modern puppetlabs_spec_helper, waiting to see if it broke any of the other tests.

@reidmv
Copy link
Contributor Author

reidmv commented Dec 21, 2020

@WhatsARanjit latest commit should fix the tests. I went through and updated the syntax of the small number of existing tests to use rspec mocks, rather than mocha, and unpinned some ancient gem versions. Tests should now pass.

Update all mocking to rspec mocks, update gem versions
There should be no need to define a special insync? method for the
classes and data properties, given that we set `should` to an
appropriate value, and Ruby compares hashes in a way such that key order
is irrelevant.
For easier reading
@WhatsARanjit WhatsARanjit merged commit 4aefd60 into puppetlabs:master Dec 27, 2020
@reidmv
Copy link
Contributor Author

reidmv commented Dec 29, 2020

@WhatsARanjit thanks!! Do you think we could get a new module release with this feature? 🙂

@reidmv reidmv deleted the merge_behaviour branch January 7, 2021 03:15
@reidmv
Copy link
Contributor Author

reidmv commented Jan 12, 2021

@WhatsARanjit ping

Btw, there's some neat Forge publish automation a colleague did a short bit ago that lets you auto-update the metadata.json file with a new version number, commit it, update the release tag, and push to the Forge all through a Github Action. Details here. It makes releasing a new version as easy as creating a new release in the Github UI.

@WhatsARanjit
Copy link
Contributor

@reidmv
Copy link
Contributor Author

reidmv commented Jan 13, 2021

W00t!!! Awesome, thanks @WhatsARanjit!! 😄

ody added a commit to ody/puppet-node_manager that referenced this pull request Feb 9, 2021
	With the merge of puppetlabsGH-60 adding node_groups fails with a schema
	error when it sends the purge_behavior parameter and is rejected
	by classifier.

	This commit adds purge_behavior to the list of filtered
	parameters that are built into the data structure sent to the
	classifier API.
WhatsARanjit pushed a commit that referenced this pull request Feb 10, 2021
With the merge of GH-60 adding node_groups fails with a schema
	error when it sends the purge_behavior parameter and is rejected
	by classifier.

	This commit adds purge_behavior to the list of filtered
	parameters that are built into the data structure sent to the
	classifier API.
@Ramesh7 Ramesh7 added the feature label Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants