553726 - Figure out HOSTNAME issues in settings_local (collection icons don't always show up).

Status: RESOLVED WONTFIX

(addons.mozilla.org Graveyard :: Code Quality, defect, P3)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Some collection icon images aren't showing up on https://preview.addons.mozilla.org/z/en-US/firefox/ (see screenshot), because of "HOSTNAME" issues, according to Wil:

"We need to load settings_local before settings...because it makes more sense to merge defaults into your settings than the other way around"

The effect of this is that image paths are like so:

http://pm-app-amo24.mozilla.org/en-US/firefox/images/collection_icon/4/1262723594, and are 404s.

Comment 1

[Wil Clouser [:clouserw]]

This is because SITE_URL is used in settings.py when it still holds a bad value.  In a perfect world, settings.py should load the local settings, and then just fill in anything that hasn't been set with default values.

A cheesy hack has been suggested:

settings.py:
 ADDON_ICON_PATH = "%s/%s/%s/images/addon_icon/%%d/%%s"
 def xx(env):
   env['ADDON_ICON_URL'] = env['STATIC_URL'] + env['ADDON_ICON_PATH']

settings_local.py:
 xx(globals())
 print ADDON_ICON_URL

Comment 2

[Wil Clouser [:clouserw]]

:)

Comment 3

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

Hmm... I'd rather do something like:


settings_pre.py <-- git ignored
settings.py <-- checked in
settings_post.py <-- git ignored

Then we can define things in settings_pre.py that need to happen ahead of time:

settings_pre.py
HOSTNAME='preview.a.m.o'

settings.py

try:
  from settings_pre import *
except ImportError:
  pass

if not HOSTNAME:
  HOSTNAME = 'whateverisdefault'

settings_post.py:
  # any overrides we need to do, e.g. DEBUG values, etc.

Not having an override-able conf would be a bad thing.

Comment 4

[Wil Clouser [:clouserw]]

If settings.py has all the "if not x: x=..." lines, how does settings_post help you?  Everything is override-able by putting it in settings_pre (or, settings_local in our case).

Comment 5

[Dave Dash [:davedash, :dd] (assign all bugs to mbrandt)]

Hmm... okay that makes sense.

Comment 6

[Wil Clouser [:clouserw]]

maybe someday we'll figure out a way not to have to override everything in settings_local that changes.  In the meantime, we've lasted this long without it and this bug is way too specific to track it.