Code review comment for lp:~adeuring/charms/precise/juju-reports/juju-reports-new-revno-in-new-dir

Thanks for explaining why we need to use symlinks here.

Thanks for switching to EAFP. I disagree that Look Before You Leap vs Easier to Ask Forgiveness than permission is simply a matter of taste; EAFP is more correct because it does not have race conditions, and typically more efficient. In the symlink case, the symlink could be moved or deleted between the os.path.lexists, os.path.islink and os.readlink calls, so EAFP is less correct. It also requires three times as many system calls, so it is less efficient.

Now, it's true that two extra system calls don't really matter. But using three times as many system calls in order to be less correct does not make sense to me.

It also looks like you're pulling the branch from scratch, instead of taking advantage of the data in the pre-existing branch. That's going to make updates slower. Instead, you could use a shared repository, or you could branch from the old branch instead of using "bzr init".

I am also concerned that update_source is now going to do work for every config change. It was intended to only do work when the branch revision changed. Do we need to start by comparing revision-ids at the top of update_source and exiting early if there's no change?