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

> 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.

I think I just figured that it was specific to the context, not something that could be generalized usefully. It also does sys.exit, which would be poor behaviour for a library function (and you can't return in a context manager).

> In that same function, I see we login as the bot.

It actually just sets the Launchpad user ID. It's not like Launchpadlib. It doesn't establish OAuth credentials or anything.

> 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.

The actual secret is the SSH key, in this case. We can delete it after pulling, if we think that's useful. But it's security-by-obscurity-- the SSH key will still be accessible in the box, just not on the filesystem.

To embrace the principle of least privilege, we'd want to grant this machine access only to the one branch it needs. But that means a separate account, and I don't like doing that.

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

Oh, it's not marked as deprecated in the Python 2 docs, but I see it's not in Python 3. Learn something every day. I'll change it.