Issue 2301 - Improve beta.adblockplus.org configuration

Issue 2301 - Improve beta.adblockplus.org configuration

[Wladimir Palant, 2015-04-15 12:23:28]

http://codereview.adblockplus.org/4808447505727488/diff/5629499534213120/modu...
File modules/web/templates/site.conf.erb (right):

http://codereview.adblockplus.org/4808447505727488/diff/5629499534213120/modu...
modules/web/templates/site.conf.erb:16: # Match Accept-Language header against
available languages
This comments makes little sense now, I'll change it into "Parse Accept-Language
header"

[Wladimir Palant, 2015-04-15 12:26:54]

http://codereview.adblockplus.org/4808447505727488/diff/5629499534213120/modu...
File modules/web/templates/adblockplus.org.conf.erb (right):

http://codereview.adblockplus.org/4808447505727488/diff/5629499534213120/modu...
modules/web/templates/adblockplus.org.conf.erb:175: rewrite ^
https://share.adblockplus.org/$lang/? redirect;
While this works right now, the approach will have to change once
share.adblockplus.org moves to its own server. I think we will just redirect to
https://share.adblockplus.org/$arg_lang/ unconditionally and let the other
server figure out whether the language exists or an additional redirect is
required.

[mathias, 2015-04-16 09:15:49]

In my opinion, there is way too much stuff done in the Nginx config that would
be better placed inside application logic. But utilizing Nginx in such fashion
is obviously quite common in this repo, so I won't start a general discussion
here.

Also, there are some nits regarding e.g. the use of concat::fragment instead of
sitescripts::configfragment, or admins@adblockplus.org etc. But this is
consistent with the existing part of the files and will be addressed in another
context.

To make a long story short; it looks good for me for now -assuming the changes
you've announced yourself are in the patch-set.

[Wladimir Palant, 2015-04-16 11:58:55]

On 2015/04/16 09:15:49, matze wrote:
> that would
> be better placed inside application logic.

What application? We are dealing with static files here.

> To make a long story short; it looks good for me for now -assuming the changes
> you've announced yourself are in the patch-set.

They are, this config is actually significantly simpler than the one currently
on server16.

[mathias, 2015-04-16 12:04:44]

LGTM.

On 2015/04/16 11:58:55, Wladimir Palant wrote:
> On 2015/04/16 09:15:49, matze wrote:
> > that would
> > be better placed inside application logic.
> 
> What application? We are dealing with static files here.

Still I prefer to avoid interpreting e.g. user-agents in server configurations
for any purpose other than security-related ones. But as I wrote before, I will
not make this a topic here. And since I am very aware that this boils down
primarily to personal preference, I will probably never make it a topic for
static content anyway.

> > To make a long story short; it looks good for me for now -assuming the
changes
> > you've announced yourself are in the patch-set.
> 
> They are, this config is actually significantly simpler than the one currently
> on server16.

Agreed.