Code review comment for lp:~jjo/charms/trusty/glance-simplestreams-sync/use-lockf-to-fix-stale-pidfiles_and_support-juju-proxy-settings

Revision history for this message
Mike McCracken (mikemc) wrote :

Thanks for the changes! Sorry this took so long to get to.

This charm is currently maintained on github, so I'm going to reject this MP and incorporate the changes there.

The lockf change will go in as-is.

I'm going to alter the change to support proxies though - I'd prefer to just have the python script look for the env file instead of adding another wrapper. I am surprised that the wrapper actually works as-is, because cron doesn't like dashes in names (see https://github.com/Ubuntu-Solutions-Engineering/glance-simplestreams-sync-charm/commit/c93f8b96e56de7aaa8e84a87c024f6d091137a79 ).
Also, the wrapper as-is doesn't wrap the 'fastpoll' cron job, which still calls the python script directly and will not get the env.

review: Disapprove

« Back to merge proposal