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

Reactor: fix traceback when salt:// path is nonexistant #38875

Merged
merged 2 commits into from Jan 24, 2017

Conversation

terminalmage
Copy link
Contributor

The get_file() function is what is run when a minion requests a file
using cp.cache_file. The expected return data from cp.cache_file is a
string, not a bool. This is likely the root cause of #36121.

@cachedout
Copy link
Contributor

There are two tests here that look like they were written to expect a bool.

@terminalmage
Copy link
Contributor Author

@cachedout I'll take a look at them. If it is indeed the case that a False result is needed in those instances, we will need to modify cp.cache_file such that it returns an empty string when the result is False. But it might also be a case of writing the test to fit the behavior. I'll figure out which is the case.

@cachedout
Copy link
Contributor

@terminalmage Many thanks as always.

When path does not exist, cp.cache_file returns False, which will cause
a traceback when we attempt to glob that path.
@terminalmage terminalmage changed the title Don't return bool when caching fails in the RemoteClient Reactor: fix traceback when salt:// path is nonexistant Jan 23, 2017
@terminalmage
Copy link
Contributor Author

@cachedout I looked back and it seems like cp.cache_file was modified for 2015.5.0 to return False instead of an empty string. So, I'm not convinced that this is the best change. I talked to Tom and he agreed, so I removed that commit and instead modified the reactor.

@cachedout cachedout merged commit b5df104 into saltstack:2016.3 Jan 24, 2017
@terminalmage terminalmage deleted the issue36121 branch April 25, 2017 19:21
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.

None yet

2 participants