Hi Chris,
We’ve got a very large backlog due to massive levels of interest in Ansible and are working through the pull request queue. It may be some time before we get to this one, and I can’t make assurances that this will be in 1.5.
It should probably not have it’s own class to just define a static method, but rather a function within LinuxNetwork.
You should also implement the check as requested to see if the file does not exist and return. You could also check in advance if the file was readable too.
try/except logic makes the code less explicit.
If we can anticipate an error and know when it will happen we always check for the error first.
I think if we can’t read it we should also set the return to None versus empty list.
Hi Michael,
You closed this patch for no apparent reason just 4 days after you emailed me, despite having left it open for weeks on GitHub. As you pointed out in a previous email, people have many items ongoing, so it would seem reasonable to provide a little time. Furthermore, I had already addressed these issues in the GitHub comments which you have failed to respond to.
As far as the try/except code, you are simply wrong. I know that no one on the message boards dares to disagree with you with regards to code/architecture. Nonetheless, you’re wrong and here’s why: No matter how many checks are made as to whether the file exists and is readable, reading CAN STILL FAIL. Even if the file is readable the microsecond before read is called, reading can still fail. Hence the existence of a try/catch semantic. Given that the try/except code is mandatory, adding additional checks before the try/except block make the code less readable, since they imply that the conditions of those checks (namely file missing or unreadable) can never fail within the try/except block, when, in fact, precisely the opposite is true.
If you’re reading files without the protection of a try/except block, you’re asking for code to fail, and fail hard in unnecessary and unexpected ways. While I claim no special expertise on python, it seems more pythonic and clearer to use the excellent features the language provides us instead of using C-isms in python.
As to the location of the code, it is expressly externalized as it applies to both BSDNetwork and LinuxNetwork. Proper software design then suggests that the code should be externalized instead of duplicated in two separate places.
This code is good, necessary and part of a pull request. Do the right thing and include it.
All the best,
~ Christopher
you can use the existing “get_file_content” function to read the file, no need to add your own.
You might want to update the function to have exception handling.
If someone has a disagreement with a comment, please email me personally, rather than the entire mailing list.