Merge lp://staging/~doanac/uci-engine/apt-mirror-support into lp://staging/uci-engine

PASSED: Continuous integration, rev:415
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/461/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/461/rebuild

Minor nit:

15 + with open(fname) as f:
16 + lines = f.readlines()
17 + with open(fname, 'w') as f:
18 + for line in lines:
19 + f.write(line.replace('archive.ubuntu.com', mirror))

Why not reading the file all at once and replace there ?

Any number to share about that improvement ?

Do we really want to do this as part of uci-engine? I think it would make more sense to bake it into the images, so juju could use it. Perhaps we need a small program to bake our images, given a base of a Canonistack daily? I also want to bake in git and the common packages to all of our components so apt-get install in a charm is a no-op for many cases.

On 03/27/2014 08:49 AM, Evan Dandrea wrote:
> Review: Disapprove
>
> Do we really want to do this as part of uci-engine? I think it would
> make more sense to bake it into the images, so juju could use it.
> Perhaps we need a small program to bake our images, given a base of a
> Canonistack daily? I also want to bake in git and the common packages
> to all of our components so apt-get install in a charm is a no-op for
> many cases.
>

You are bringing up a 2nd issue which is somewhat different from what
I'm addressing here. The images Juju sets up use cloud-init and provide
the proper apt-mirror info (although the base image they use is missing
the stuff you mentioned). This MP is updating the image that the engine
is building for the user which is a configurable value they provide us.
In our case we weren't doing the proper cloud modifications to the image
we built so ours wasn't working correctly. ie - This MP is about making
the system-under-test work better not our infrastructure systems.

On 03/27/2014 08:46 AM, Vincent Ladeuil wrote:
> Minor nit:
>
> 15 + with open(fname) as f:
> 16 + lines = f.readlines()
> 17 + with open(fname, 'w') as f:
> 18 + for line in lines:
> 19 + f.write(line.replace('archive.ubuntu.com', mirror))
>
> Why not reading the file all at once and replace there ?
>
> Any number to share about that improvement ?

No - I'm just not as good of a multitasker as you. I'll make the change,
provided ev agrees this MP is still worth doing.

> On 03/27/2014 08:49 AM, Evan Dandrea wrote:
> > Review: Disapprove
> >
> > Do we really want to do this as part of uci-engine? I think it would
> > make more sense to bake it into the images, so juju could use it.
> > Perhaps we need a small program to bake our images, given a base of a
> > Canonistack daily? I also want to bake in git and the common packages
> > to all of our components so apt-get install in a charm is a no-op for
> > many cases.
> >
>
> You are bringing up a 2nd issue which is somewhat different from what
> I'm addressing here.

Thanks for clarifying that, I think ev's remark is worth its own bug but
that shouldn't block this MP.

> This MP is updating the image that the engine
> is building for the user which is a configurable value they provide us.
> In our case we weren't doing the proper cloud modifications to the image
> we built so ours wasn't working correctly. ie - This MP is about making
> the system-under-test work better not our infrastructure systems.

+1 on that, the only thing I now can think about is that there may be an
impact on users wanting to download and use that image for local testing.

But if I understand how this IP-based reply work, that should still work for
that use case.

Note that it's probably a wrong idea for the user to update this image too
much if he cares about reproducibility. But that's true for whatever mirror
they use anyway so that should not block this MP IMHO.

Ah, I understand now. Thank you for the explanation. +1