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

keep cache_copied_files variable a list #56327

Merged
merged 3 commits into from
Mar 13, 2020
Merged

keep cache_copied_files variable a list #56327

merged 3 commits into from
Mar 13, 2020

Conversation

mchugh19
Copy link
Contributor

@mchugh19 mchugh19 commented Mar 8, 2020

What does this PR do?

Retains list datatype of cache_copied_files variable

What issues does this PR fix or reference?

#56324

Previous Behavior

Capable of runtime error AttributeError: 'set' object has no attribute 'extend'

New Behavior

Datatype of cache_copied_files is consistently a list

Tests written?

Yes

Commits signed with GPG?

No

@mchugh19 mchugh19 requested a review from a team as a code owner March 8, 2020 10:58
@ghost ghost requested a review from xeacott March 8, 2020 10:59
@mchugh19
Copy link
Contributor Author

mchugh19 commented Mar 8, 2020

Minor fix with no change in functionality. Would be nice to get into a backport.

@garethgreenaway garethgreenaway added the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Mar 8, 2020
@garethgreenaway
Copy link
Contributor

@mchugh19 Thanks for the fix. Would be great to get a testcase on this one to prevent regressions.

@mchugh19
Copy link
Contributor Author

@garethgreenaway tests added! Looks like the issue can be replicated with includes. I've validated the issue is present with the saltcheck integration test without the fix, and corrected with the patch. This should be good to go!

@mchugh19
Copy link
Contributor Author

@Ch3LL it'd be nice if this could make 3000.1

@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 12, 2020

let me update the branch, and have the tests run and see about getting it in. i'll let ya know

@Ch3LL Ch3LL added v3000.1 vulnerable version and removed Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases labels Mar 13, 2020
@dwoz dwoz merged commit d8fc072 into saltstack:master Mar 13, 2020
@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 13, 2020

@mchugh19 been merged and will be included in 3000.1. thanks for the headsup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3000.1 vulnerable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants