-
-
Notifications
You must be signed in to change notification settings - Fork 434
Fix file content check to handle missing files #1461
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
base: 3.x
Are you sure you want to change the base?
Conversation
Not sure if the discussion is better in #1454 or here. I am not sure this is entirely correct now. Take the case where the file does exist but is empty. For instance something like: Also, I think this would be better: def process(self, output):
if not output:
return self.default()
return output ... if I understood the logic of FactBase correctly. |
@NichtJens You are right, fixed it |
@wowi42 Does this already take my other question into account?
|
Nope, I was reading too quickly. I'm unsure how to differentiate between no file and an empty file. Perhaps we should involve @Fizzadar for his input. |
Yes, this is a problem. Actually, it's exactly why I didn't propose a PR similar to yours and just opened an issue instead. I should probably have specfied what I had tested already. Sorry about that. Btw., from my experiments, you can go back to the old: def process(self, output):
return output ... and it works the same as your current code. The magic happens in In fact, the reason I am asking is since I have the same problem with what I am currently working on. This was also the reason I posted the issue in the first place. I need to update the contents of a json file on the remote side. For this, I wanted a fact that gets the json contents and came up with this: class JSONFileContents(files.FileContents):
"""
Returns the contents of a JSON file as dict.
Returns an empty dict if the file does not exist or cannot be parsed as JSON.
"""
default = dict
def command(self, path):
return make_formatted_string_command("cat {0} || true", QuoteString(path))
def process(self, output):
output = super().process(output)
output = "\n".join(output)
try:
output = json.loads(output)
except json.decoder.JSONDecodeError:
return self.default()
return output This is very similar to yours, except I just kept the I think these two changes make the PR very minimal (single line change, plus tests). But I am still not sure what the desired behavior really is. |
The expected behaviour is:
The |
Fix #1454
3.x
at this time)scripts/dev-test.sh
)scripts/dev-lint.sh
)