← Back to team overview

maas-devel team mailing list archive

Re: Longpoll support

 

> [...]
> -            r'^%s$' % re.escape(django_settings.LONGPOLL_URL),
> +            r'^%s$' % re.escape(django_settings.LONGPOLL_SERVER_URL),
>              proxy_to_longpoll, name='proxy-to-longpoll'),
>          )

I understand that you did that change to quickly fix the problem.  I'll
revert that change and fix the problem properly in
https://code.launchpad.net/~rvb/maas/maas-longpoll-fix/+merge/97582.

I'm really sorry about that problem.  I committed a change to demo.py
(the demo configuration settings) that led to a conflict in the demo
config.  I've added a proper fix and a proper test for that in the
branch mentioned above.

> But after inspection it seems that settings has both LONGPOLL_SERVER_URL
> and LONG_URL which is _terribly_ confusing. So I don't have confidence
> in my patch.

I'm aware that it is a little bit confusing but there is no way around
having two configuration options.  Note that this confusion won't be
exposed to the user because these settings should not be exposed (or
changed) in the package's configuration file.

FTR:

LONGPOLL_SERVER_URL is the url where a txlongpoll server can be reached.
 This is only required in dev mode because only in this case we need the
django app to act as a proxy to txlongpoll.  In production, apache
should do that.

LONGPOLL_URL is the relative url where a longpoll server can be reached
by the JS longpoll module (usually '/longpoll').

> Also, the fact that we are erroring in urls.py means that we are lacking
> a test here for this code path.

You're right there, nothing should blow up the way it did, even with
wrongly configured settings.  I've added a fix and a test for that (see
above).

Again,I'm really sorry about that problem, I've landed all my
longpoll-related branches all at once yesterday.  I'll try to commit
stuff more incrementally in the future.

Raphaël


Follow ups

References