On Thu, 2011-12-08 at 12:17 +0000, Paul Sokolovsky wrote:
> Hello,
> 
> On Wed, 07 Dec 2011 13:38:32 -0000
> Guilherme Salgado <email address hidden> wrote:
> 
> > > > I don't quite like the commented code left behind nor the
> > > > hard-coded paths. The former is not a big deal but we should at
> > > > least try and fix the latter
> > > 
> > > Well, all the changes here are essentially configuration for
> > > running specific instance of service on specific machine, so yes,
> > > all paths are specific to that machine. We could have two configs
> > > for production vs reserve/test machine, but then they would need to
> > > be updated in parallel, so as they differ in just 3-4 lines, it's
> > > easier to keep them in one file, to avoid head- scratching trying
> > > to understand what's where in 6 months ;-).
> > 
> > runner.sh doesn't look like configuration to me; it looks like a
> > script which developers may want to run on their machine and in that
> > case they'll have to manually change the script. 
> 
> Well, that's typical pattern of setting up a cronjob: instead of
> putting too long lines into crontab itself, you call a wrapper script
> which sets up environment/configuration for actual (and generic) tool
> to run. So, it's kinda of "executable configuration". runner.sh is not
> intended to be run by developer, except for the case of simulating
> running from cron (that's another case why this pattern of wrapper
> scripts is useful). So, maybe the name is confusing, let's rename it to
> cron-runner.sh and add corresponding comment if that answers your
> concerns.
> 
> > I see that
> > sync-linaro.sh also has a hard-coded path in it, so if I wanted to
> 
> Well, that's a deprecated script ;-). It really should be removed some
> time later, once new system is up and running for some buffer time.
> 
> > test linaro-android-gerrit-support on my machine I'd have to change
> > at least these two places and I'd have to find a way to keep those
> > changes locally while I'm trying l-a-g-s, but not commit them back.
> > We used to have a similar problem in Offspring and I can tell you
> > it's very annoying.
> 
> Yes, I understand your concerns. But those are indeed just one-off
> wrapper scripts. Except that one "one-off" is special - the production

What do you mean with one-off here?  To me a one-off script is something that
we'll run once and throw away, but here it's something that will run as a
cronjob in production, so it definitely doesn't fall on that category.

> one. It could have been a separate script lying elsewhere on a target
> host, but we really want to capture all details of the production
> service setup, to be able to re-setup it without contingencies in case

Ok, if that's not expected to be needed on developer machines it's probably
fine to keep the paths hard-coded, but it's a good idea to move it to a
separate directory, which you've already did. :)

> of need. The ideal situation to handle that would be to keep generic
> tool vs specific service config in two different repositories, and I
> would happily do that if LP supported >1 repos per project. But
> creating a new LP project for each such case doesn't scale at all IMHO.

You can have as many branches as you want under a given project in LP and
those branches may contain distinct code bases, so you could do something
like:

   mkdir cronscripts
   cd cronscripts
   bzr init
   cp /path/to/wrapper-script .
   bzr add *
   bzr commit -m "..."
   bzr push lp:~linaro-infrastructure/linaro-android-gerrit-support/cronscripts

And then when deploying you fetch that branch as well.

> Anyway, if you think that separating generic tool vs specific config
> into different dirs in repo will be helpful, let's do that.
> 
> Finally, answering your concern how to deal with that locally - you'd
> need just to copy runner.sh to your one-off wrapper, runner-local.sh,
> and change it, that's the idea.

Yes, I know there are many ways a developer can deal with that, but my point
is that they shouldn't have to change any code to run (or test) our software,
and if they do need to change configs and such, it should be documented.