968600 - config/*.ini-dist files are out-of-date

Status: RESOLVED INVALID

(Socorro :: Backend, task)

Description

[Robert Helmer [:rhelmer]]

I hit this trying to configure my database host - the parameter has changed to "hostname"

I think it is convenient to have these checked in, but maybe we should switch the docs to insist on generating the files as-needed e.g.:

middleware_app.py --admin.print_conf=ini > config/middleware.ini

If not, then I suggest that we add a test to jenkins that does the above (except to e.g. middleware.ini-dist) then balks if git shows any modifications to the file. This would catch somebody modifying the defaults and forgetting to update the -dist file.

Comment 1

[Peter Bengtsson [:peterbe]]

FYIW, it was a mistake to dump "default" configs when they had their default values set. We agreed during the Stability work week that we should do it like the likes of many other Unix programs where defaults are left in code and the config file looks something like this::

  # Change this to get more unicorns
  #pink_unicorns=false

I really don't know where to start actually doing this we should hunt down any config files we're using that have the defaults actually set in config (and not just shown commented out).

Comment 2

[Robert Helmer [:rhelmer]]

(In reply to Peter Bengtsson [:peterbe] from comment #1)
> FYIW, it was a mistake to dump "default" configs when they had their default
> values set. We agreed during the Stability work week that we should do it
> like the likes of many other Unix programs where defaults are left in code
> and the config file looks something like this::
> 
>   # Change this to get more unicorns
>   #pink_unicorns=false
> 
> I really don't know where to start actually doing this we should hunt down
> any config files we're using that have the defaults actually set in config
> (and not just shown commented out).

Having them commented-out seems fine to me, but it's still important that they are not out-of-date - for example right now middleware.ini-dist has this:

  database_host='localhost'

However that has changed and should now be:

  database_hostname='localhost'

It would still not work if somebody were to uncomment it.

FWIW, if I --admin.print_conf=ini right now it looks like everything is commented-out by default.
So I'd be ok if we at least re-generated and checked these in, ideally along with a test that catches if they go out-of-date again like described in the second half of comment 0.

Comment 3

[Peter Bengtsson [:peterbe]]

(In reply to Robert Helmer [:rhelmer] from comment #2)
> (In reply to Peter Bengtsson [:peterbe] from comment #1)
snip
> 
> FWIW, if I --admin.print_conf=ini right now it looks like everything is
> commented-out by default.
> So I'd be ok if we at least re-generated and checked these in, ideally along
> with a test that catches if they go out-of-date again like described in the
> second half of comment 0.

+1

Comment 4

[(not currently active) Ted Mielczarek]

Not sure if bug 935766 is a dupe of this or the other way around. Either way, it would be great if the .ini-dist files didn't have outdated bits so they'd match what we're doing in production (where that makes sense). Configuring a local Vagrant VM I found that processor.ini-dist is set to use the LegacyCrashProcessor and not the new stackwalker with JSON output.

Comment 5

[Lonnen :lonnen]

this bug is evergreen