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

fix(map.jinja): _merge_ defaults and config.get #115

Merged
merged 1 commit into from
May 15, 2019

Conversation

alxwr
Copy link
Member

@alxwr alxwr commented May 15, 2019

The merge which was formerly done by pillar.get(..., merge=True) has now to be performed by grains.filter_by.

Tested on FreeBSD with both salt and salt-ssh.

@alxwr alxwr requested a review from myii May 15, 2019 16:33
@myii
Copy link
Member

myii commented May 15, 2019

Follows on from #102 and #114.

Unfortunately, still triggering an error on my setup:

RuntimeError: maximum recursion depth exceeded while calling a Python object

So we've been communicating on Slack/IRC to look at an alternative solution.

@alxwr alxwr force-pushed the fix-merge branch 4 times, most recently from 2f41ae0 to f71ff87 Compare May 15, 2019 18:42
@alxwr
Copy link
Member Author

alxwr commented May 15, 2019

@myii: fixed the merging, config.get gets only called once. Added tests to verify a correct merge.

@myii
Copy link
Member

myii commented May 15, 2019

Final solution for salt-ssh!

pillar.example Outdated

# Just for testing purposes
winner: pillar
added_in_pillar: asdf
Copy link
Member

Choose a reason for hiding this comment

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

Just to make it clear that is being pulled from the correct place, wouldn't it be better to have a different value than the one added_from_lookup ? ie, added_in_pillar: pillar_value?

Copy link
Member Author

Choose a reason for hiding this comment

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

@javierbertoli I'll update the PR. Thanks for the feedback!

config: '/etc/template'
service:
name: template
# Just here for testing
added_in_defaults: asdf
Copy link
Member

Choose a reason for hiding this comment

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

same here?

@myii myii merged commit aa008e6 into saltstack-formulas:master May 15, 2019
@alxwr alxwr deleted the fix-merge branch May 15, 2019 19:46
@saltstack-formulas-travis

🎉 This PR is included in version 2.1.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

myii added a commit to myii/template-formula that referenced this pull request May 27, 2019
* To distinguish between:
  - `salt-minion`
  - `salt-call`
  - `salt-ssh`
* Invoked like `map.jinja`:
  - `{%- from tplroot ~ "/libsaltcli.jinja" import cli with context %}`
* Based upon work done in PRs: saltstack-formulas#102, saltstack-formulas#114 & saltstack-formulas#115
myii added a commit to myii/template-formula that referenced this pull request Mar 25, 2020
* To distinguish between:
  - `salt-minion`
  - `salt-call`
  - `salt-ssh`
* Invoked like `map.jinja`:
  - `{%- from tplroot ~ "/libsaltcli.jinja" import cli with context %}`
* Based upon work done in PRs: saltstack-formulas#102, saltstack-formulas#114 & saltstack-formulas#115
* Finalised from saltstack-formulas/libvirt-formula#71
* Required by saltstack-formulas#186
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.

4 participants