Code review comment for lp:~brian-murray/ubuntu-archive-tools/phased-updater

Revision history for this message
Steve Langasek (vorlon) wrote :

+def set_pup(new_pup, suite, src_pkg):
+ import subprocess
+ with open(os.devnull, 'w') as fnull:
+ retcode = subprocess.call([
+ "%s/change-override" % os.getcwd(),
+ "--confirm-all",
+ "--percentage", '%s' % new_pup,
+ "--suite", suite,
+ "--source-and-binary", src_pkg,
+ "--launchpad", options.launchpad_instance],
+ stdout = fnull)
+ if retcode != 0:
+ logging.error("There was an error (%s) running change-override."
+ % retcode)
+ return retcode

os.getcwd() doesn't do what you want here in all cases. I, for example, have ubuntu-archive-tools on my path, so my getcwd() returns a location completely unrelated to where the tools are.

Instead, I think you want:

  os.path.dirname(sys.argv[0]) or "."

The other use of os.getcwd() seem fine, as they refer to the output directory and I think it's reasonable to use the current dir as the convention for that. But in production, I think we aren't going to be writing the report to the checkout dir.

Otherwise, this looks good to me and ready to merge.

review: Needs Fixing

« Back to merge proposal