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

Revision history for this message
Eric Chen (eric-chen) wrote :

I'm so surprise that we will execute /var/lib/juju/agents/unit-logrotated-1/charm/lib/lib_cron.py from the cronjob. It is very unreasonable to me. I suppose that every library should be used during

- event handler under src/reactive
- juju actions under src/actions

Could you check the reason why we need to design like this?

BTW, update_cron_daily_schedule is not only used in /etc/cron.daily/charm-logrotated, it may be triggered from

src/actions -> update_cronjob -> cron.install_cronjob -> update_cron_daily_schedule

The runner of these two behaviors may be different. Could you also confirm it?

1. from cron.daily
2. from juju actions

review: Needs Information

« Back to merge proposal