It's dangerous for the logic when we want to modify it in the future.
The hidden logic here is.. we only put the cronjob file with one frequency ["hourly", "daily", "weekly", "monthly"]. That is the design limitation in charm-logrotate, not a limitation in linux cronjob. But I don't have good idea to make this design limitation explict in the cleanup_cronjob function right now.
I have other ideas to remove some duplicated code and avoid additional parameter in the cleanup_cronjob.
1. There could be a cronjob filepath generator to be used in install_cronjob and cleanup_cronjob
2. We can use frequency index to generate the path and it's easy to know which one should be reserved in the cleanup_cronjob. We also can change the parameter as an array in the future if need.
/* Pseudo Code */
def cleanup_cronjob(self, reserve=-1):
'''
if reserve = -1, delete cronjob file in all frequencies
else: only reserve the cronjob file in corresponding folder.
'''
for i in range(len(self.cronjob_check_paths)):
if reserve == -1 or i != reserve:
path = cronjob_path_generator(i) os.remove(path)
It's dangerous for the logic when we want to modify it in the future.
The hidden logic here is.. we only put the cronjob file with one frequency ["hourly", "daily", "weekly", "monthly"]. That is the design limitation in charm-logrotate, not a limitation in linux cronjob. But I don't have good idea to make this design limitation explict in the cleanup_cronjob function right now.
I have other ideas to remove some duplicated code and avoid additional parameter in the cleanup_cronjob.
1. There could be a cronjob filepath generator to be used in install_cronjob and cleanup_cronjob
2. We can use frequency index to generate the path and it's easy to know which one should be reserved in the cleanup_cronjob. We also can change the parameter as an array in the future if need.
/* Pseudo Code */ cronjob( self, reserve=-1):
def cleanup_
'''
if reserve = -1, delete cronjob file in all frequencies
else: only reserve the cronjob file in corresponding folder.
'''
for i in range(len( self.cronjob_ check_paths) ): path_generator( i)
os. remove( path)
if reserve == -1 or i != reserve:
path = cronjob_