Skip to content

Fix salt 2019.2 breaking change using yaml filter#50

Merged
myii merged 2 commits into
saltstack-formulas:masterfrom
necabo:master
Jul 16, 2019
Merged

Fix salt 2019.2 breaking change using yaml filter#50
myii merged 2 commits into
saltstack-formulas:masterfrom
necabo:master

Conversation

@necabo

@necabo necabo commented Jul 16, 2019

Copy link
Copy Markdown
Contributor

With the salt 2019.2.0 (Fluorine) release there has been a non-backward-compatible change to the yaml renderer.

This PR addresses that change by using the yaml filter where necessary.

@vutny vutny left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, @necabo
Similar patches were submitted to some repos in Salt Formulas, it's good we have one more formula compatible with latest stable Salt release.

@myii

myii commented Jul 16, 2019

Copy link
Copy Markdown
Contributor

Thanks for this PR, @necabo.

@vutny In terms of consistency with the other fixes, would it be appropriate to use | json instead? Or is | yaml required specifically in this case?

@necabo

necabo commented Jul 16, 2019

Copy link
Copy Markdown
Contributor Author

Or is | yaml required specifically in this case?

I can assure that both | json and | yaml work for the pillar I tested with (example pillar).

@myii

myii commented Jul 16, 2019

Copy link
Copy Markdown
Contributor

@necabo Thanks for testing that. There is a long history of changes related to this issue and the consistent option we have selected is to use | json. Of course, there are situations where | json doesn't work and we will use | yaml instead. Unless that's the case here, would you mind changing this PR to use | json instead? That will help us come back to these, when the eventual solution of | tojson needs to be considered for implementation. Refer back to saltstack-formulas/consul-formula#40 (comment) and the linked content there for more info.

@myii myii merged commit bafb2e9 into saltstack-formulas:master Jul 16, 2019
@myii

myii commented Jul 16, 2019

Copy link
Copy Markdown
Contributor

@necabo @vutny Merged, thanks to you both.

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.

3 participants