Code review comment for lp:~ursinha/uci-engine/private-lp-mthood

Revision history for this message
Andy Doan (doanac) wrote :

On 05/07/2014 12:12 PM, Ursula Junque wrote:
> Thanks for reviewing, Andy.
>
>>> === modified file 'branch-source-
>> builder/cupstream2distro/launchpadmanager.py'
>>
>>> +sys.path.append(os.path.join(os.path.dirname(__file__), '../ci-utils'))
>>
>> I don't think we need sys.path.append stuff in our code anymore.
>
> This branch is on top of mthood one, that is a tad far from lp:uci-engine/trunk. Are these things you mention available since when? I think this should be changed in case we're merging this with trunk, otherwise I'm not sure it's going to work? (same apply for other comments regarding things we have now and didn't have when this was created).

yes. it was added here:

<http://bazaar.launchpad.net/~canonical-ci-engineering/uci-engine/mthood/revision/320>

>>> +web_root ="https://mthoodapi.lacinonac.com/"
>>
>> I don't understand the details well enough, but should this value come
>> from our unit_config rather than being hard-coded? Actually it looks
>> like deploy.py might be setting this in the unit_config?
>
> This script is only a helper to create the credentials to the private lp instance, I think in trunk the original create_lp_creds.py doesn't even exist anymore. I agree this could be done using values on unit_config, but as this is something that should go away soon I didn't bother much.

ah - okay. makes sense then.

« Back to merge proposal