Code review comment for ~dashmage/charm-logrotated:bug/2017798-update-cron-daily-location

I cannot fully understand your answer above. I want to know why we need to execute cron_lib.py regularly. But I didn't find the real reason in your explanation.

My major concern is

1. In general case, the library in charm is used under the juju framework. The design in logrotate breaks the common situation and we should know the real reason. Maybe the solution is not to copy the file, we should just avoid this behavior.

2. Sudo is a dangerous command, we should only use it when needed. We should also need to know how the executor gets the permission?

3. subprocess is always my last choice to handle the program. because it implies many assumptions of the environment. So I agree with Tianqi's comment, we should consider the mature library. Even when we copy the file, we should think about how to use shutil.move if possible. (In the previous commit, you didn't use shutil.move when there is no "sudo").

4. "Retry" is also something I don't like a lot. Please write down the reason why we cannot avoid it. If there is a bug that only happen with 30% possibility, it will be concealed by the "Retry" mechanism.

I suggest that you can write a spec to clarify all the things and purpose the proposal again.