Adds __env__ substitution to ext_pillar.stack
What does this PR do?
- Adds
__env__substitution to ext_pillar.stack; followup of #61531 - Improved exception handling for stacked template (jinja) template rendering and yaml parsing in ext_pillar.stack
New Behavior
adds __env__ to stacked pillar config
ext_pillar:
- stack:
opts:pillarenv:
dev: /path/to/dev/stack.cfg
__env__: /path/to/__env__/stack.cfg
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
- [x] Docs
- [x] Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
- [x] Tests written/updated
Commits signed with GPG?
No
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.
@nicholasmhughes: I think this code is working ok, do you agree?
can you rebase your branch against master from the start? it looks like you are pulling in changes from all over. which makes this almost impossible to review.
can you rebase your branch against master from the start? it looks like you are pulling in changes from all over. which makes this almost impossible to review.
Done.
@whytewolf: I have no idea how to proceed. The pr-opensuse-15-x86_64-py3-pytest says: Failing after 71m — Run Tests (Fast): error in 'sh' step but I don't understand how that may be causes by these changes. Can you please advise?
@whytewolf, @nicholasmhughes: the tests give some errors that I think are unrelated to my changes. I don't know how to proceed, please advise.
re-run pr-windows-2019-x64-py3-pytest
re-run pr-windows-2019-x64-py3-pytest
I've triggered a rerun with a new push, but I don't see how I can trigger a rerun otherwise. I also removed the [WIP] tag in the subject line and made updates to /docs/man/salt.7 and /changelog
As far as I can see there are no ext_pillar.stack unit tests that I can update for this __env__ behaviour.
re-run pr-windows-2019-x64-py3-pytest
I've triggered a rerun with a new push, but I don't see how I can trigger a rerun otherwise. I also removed the [WIP] tag in the subject line and made updates to /docs/man/salt.7 and /changelog
As far as I can see there are no ext_pillar.stack unit tests that I can update for this
__env__behaviour.
there may not be an existing test, that does happen. If possible would you be able to make a new test for the functionality?
there may not be an existing test, that does happen. If possible would you be able to make a new test for the functionality?
@whytewolf: I've added a stacked pillar unit test, testing the old and new functionality.
@whytewolf:
The friendly PR-robot tells me
The function 'ext_pillar' on 'salt/pillar/stack.py' does not have a docstring
That's true, but the docstring is in the top of the file. Should I move the docstring, add a dummy reference or leave this as it is?
I've filed bug report #62693 that I discovered working on this PR.
I consider this PR ready for merging in master.
@whytewolf:
The friendly PR-robot tells me
The function 'ext_pillar' on 'salt/pillar/stack.py' does not have a docstring
That's true, but the docstring is in the top of the file. Should I move the docstring, add a dummy reference or leave this as it is?
I've filed bug report #62693 that I discovered working on this PR.
I consider this PR ready for merging in master.
take inspiration for the other ext_pillar modules. the docstring for the ext_pillar function should be short and to the point of what the function does. the one at the top that covers the enter functionality should be left alone. as that is the main point of the entire module.
the one for ext_pillar should be something like Execute queries against MySQL, merge and return as a dict
@whytewolf: ok I've added the docstring, and all checks pass. So now I need a review. Can you do the code review?
Regards, Jasper
looks like there are still changes to the man pages
Oh, my bad. Fixed.
Hi! I'm your friendly PR bot!
You might be wondering what I'm doing commenting here on your PR.
Yes, as a matter of fact, I am...
I'm just here to help us improve the documentation. I can't respond to questions or anything, but what I can do, I do well!
Okay... so what do you do?
I detect modules that are missing docstrings or "CLI Example" on existing docstrings! When I was created we had a lot of these. The documentation for these modules need some love and attention to make Salt better for our users.
So what does that have to do with my PR?
I noticed that in this PR there are some files changed that have some of these issues. So I'm leaving this comment to let you know your options.
Okay, what are they?
Well, my favorite, is that since you were making changes here I'm hoping that you would be the most familiar with this module and be able to add some other examples or fix any of the reported issues.
If I can, then what?
Well, you can either add them to this PR or add them to another PR. Either way is fine!
Well... what if I can't, or don't want to?
That's also fine! We appreciate all contributions to the Salt Project. If you can't add those other examples, either because you're too busy, or unfamiliar, or you just aren't interested, we still appreciate the contributions that you've made already.
Whatever approach you decide to take, just drop a comment here letting us know!
Detected Issues (click me)
Check Known Missing Docstrings...........................................Failed - hook id: invoke - exit code: 1/home/runner/.cache/pre-commit/repo7yawj1iq/py_env-python3/lib/python3.9/site-packages/_distutils_hack/init.py:33: UserWarning: Setuptools is replacing distutils. warnings.warn("Setuptools is replacing distutils.") The function 'ext_pillar' on 'salt/pillar/stack.py' does not have a docstring Found 1 errors
Thanks again!
Hi! I'm your friendly PR bot!
You might be wondering what I'm doing commenting here on your PR.
Yes, as a matter of fact, I am...
I'm just here to help us improve the documentation. I can't respond to questions or anything, but what I can do, I do well!
Okay... so what do you do?
I detect modules that are missing docstrings or "CLI Example" on existing docstrings! When I was created we had a lot of these. The documentation for these modules need some love and attention to make Salt better for our users.
So what does that have to do with my PR?
I noticed that in this PR there are some files changed that have some of these issues. So I'm leaving this comment to let you know your options.
Okay, what are they?
Well, my favorite, is that since you were making changes here I'm hoping that you would be the most familiar with this module and be able to add some other examples or fix any of the reported issues.
If I can, then what?
Well, you can either add them to this PR or add them to another PR. Either way is fine!
Well... what if I can't, or don't want to?
That's also fine! We appreciate all contributions to the Salt Project. If you can't add those other examples, either because you're too busy, or unfamiliar, or you just aren't interested, we still appreciate the contributions that you've made already.
Whatever approach you decide to take, just drop a comment here letting us know!
Detected Issues (click me)
Check Known Missing Docstrings...........................................Failed - hook id: invoke - exit code: 1/home/runner/.cache/pre-commit/repo7yawj1iq/py_env-python3/lib/python3.9/site-packages/_distutils_hack/init.py:33: UserWarning: Setuptools is replacing distutils. warnings.warn("Setuptools is replacing distutils.") The function 'ext_pillar' on 'salt/pillar/stack.py' does not have a docstring Found 1 errors
Thanks again!
Congratulations on your first PR being merged! :tada: