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 pillar include regression #52490

Merged
merged 9 commits into from Apr 17, 2019

Conversation

@dwoz
Copy link
Contributor

commented Apr 10, 2019

What does this PR do?

  • Fixes pillar include regression while preserving wildcard include functionality.
  • Adds regression tests for both.

Fixes #52134
Wild card include issue #22063

Previous Behavior

Including from multiple files stomped on pillar key

New Behavior

Fixes key stomping regression

Tests written?

Yes

Commits signed with GPG?

Yes

dwoz added 3 commits Apr 10, 2019
This reverts commit fbab73a.
@dwoz dwoz requested a review from saltstack/team-core as a code owner Apr 10, 2019
@dwoz dwoz added the 2019.2.1 label Apr 10, 2019
@waynew

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

Hm... does this make https://github.com/saltstack/salt/pull/52008/files irrelevant? Or is this change for a different problem?

@dwoz

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

@waynew Looks like this is a separate issue. I think we should merge this change into your PR.

dwoz added 3 commits Apr 10, 2019
@waynew

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

The thing I'm most concerned about is that this PR is removing the matched_pstates (or changing it to assignment rather than .extend)

@dwoz

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

@waynew okay, I'll take a look at it again.

@dwoz

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

@waynew I took a look at this again and everything still looks right to me. Is there a test we can write that would confirm there is or is not an issue? Or would you like me to merge this into your change so you can test it?

@dwoz

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

@waynew Since you're PR's (#52008) tests have passed I'll merge your changes then update my branch accordingly

dwoz added 2 commits Apr 12, 2019
@waynew
waynew approved these changes Apr 15, 2019
Copy link
Contributor

left a comment

👍

@dwoz dwoz merged commit 9faa49c into saltstack:2019.2 Apr 17, 2019
10 checks passed
10 checks passed
WIP Ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint Python lint test has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.