Code review comment for lp:~abentley/charms/precise/juju-reports/major-updates

Revision history for this message
Curtis Hovey (sinzui) wrote :

In update_source() you use a try/except block...I expected a context manger. The lines aren't wrong, I just expect context managers for resources in code that YOU write.

In that same function, I see we login as the bot. This has bothered me for some time. We don't logout. Should we? Am I paranoid. I ask because I am pondering a jenkin-juju-ci subordinate charm and I feel safer if the charm can login to get private branches, but logs out when done. The user is not left with powers that have no legitimate need.

In install_cronjob() file() is used. The function is deprecated. Does this work?
    open('/etc/cron.d/ubuntu', 'w').write(str(t))

review: Needs Information (code)

« Back to merge proposal