Issue 3112 - Support use of sitescripts URL handlers from the test server

Issue 3112 - Support use of sitescripts URL handlers from the test server

(Depends on this change to sitescripts https://codereview.adblockplus.org/29328524/ )

[kzar, 2015-09-23 13:26:29]

Patch Set 1

[kzar, 2015-09-23 13:54:55]

Patch Set 2 : Avoid overzealous exception catching

[Sebastian Noack, 2015-09-29 11:45:31]

https://codereview.adblockplus.org/29328527/diff/29328542/cms/bin/test_server.py
File cms/bin/test_server.py (right):

https://codereview.adblockplus.org/29328527/diff/29328542/cms/bin/test_server...
cms/bin/test_server.py:106: from sitescripts.web import get_handler
I'm not too happy with this change. I don't think that sitescript specific hacks
belong into the generic CMS code. I'd rather make the handler function(s)
configurable in settings.ini.

[Sebastian Noack, 2015-09-29 12:47:40]

Dave and I discussed that approach a little more on IRC but didn't come to a
conclusion yet. For reference:

14:14:55 <snoack> I wonder whether we really need this sitescripts hack in the
CMS
14:15:52 <snoack> Note that for other websites dynamic content is just not
available when running the test server, e.g. the contact form on eyeo.com, and
the installation link email thing on adblockbrowser.org
14:16:11 <kzar> Well, I did consider doing it the way you suggested in the
review originally, but settled on my submitted solution as it kept things simple
14:16:39 <kzar> Yea, it's a problem we have been having. Ollie has asked me
about it before, Manvel as well
14:17:12 <kzar> So I would definitely like to see _some_ solution pushed, not
sure if my one is the best
14:17:20 <snoack> As I said, I'm not sure whether this scenario is worth to
support at all, regardless of the implementation, as we didn't have it for other
websites so far. And it never was a problem.
14:17:29 <kzar> It was, that's my point
14:19:05 <kzar> For example Ollie came to me a while ago and asked about it. He
ended up having to serve the static pages with the CMS test server, opening the
relevant page in the browser, then stopping the server, then launching his new
URL handler to send IE emails, then clicking the button on the page to test it
worked
14:19:46 <kzar> I've had this problem as well, the sitekey testpages handler is
just the latest example. Manvel also agreed with me that this would be useful
14:20:29 <kzar> ("this" being the ability to serve CMS generated static pages
along withe some dynamic sitescripts URL handlers at the same time from the test
server)
14:21:08 <kzar> I'm open to suggestions about the best solution but this is
definitely something that would be useful
14:25:29 <snoack> Well, it didn't strike me as a huge issue. The test server
exists to make frontend development easy. It's neither meant nor sufficient for
proper QA, testing workflows involving the backend. And there doesn't seem to be
a good solution for that anyway. We just made the CMS a standalone solution
agnostic of sitescripts, and I prefer to keep it like that.
14:25:29 <snoack> Yeah, making the handler configurable would retain that
aspect. But still, it feels like inappropriate magic. Also consider not all URLs
supported by the multiplexer are available on all domains. Some might even
clash, e.g. foo.com/foo can be handled by multiplexer while bar.com/foo can be
static.

[kzar, 2015-09-29 13:18:42]

This would be useful for me, Manvel and Ollie. We sometimes need to test how
static and dynamic pages interact - I've given a couple of examples too. I don't
mind how we implement it, I'm open to suggestions there, but I find it a little
frustrating that it's dismissed as "not a huge issue". (Of course literally it's
true that this is not a huge issue, but to me the phrasing kind of implies it
doesn't matter at all.)

The implementation as it stands is pretty simple and doesn't really have any
down sides that I can see. If you're worried about magic I would be happy to add
a warning message to explain what's going on.

[Wladimir Palant, 2015-09-29 16:21:21]

Nope, NOT LGTM. An explicit dependency on sitescripts in the CMS makes no sense:

* The CMS is a generic framework, many websites using don't depend on
sitescripts at all.

* The fact that a particular URL is handled by sitescripts means little. This
URL might be meant for a different domain, or it might need special
configuration/databases/whatever.

* We will now get different behavior in the test server depending on whether
sitescripts are installed, which revision of sitescripts and whether they are
configured properly.

There are really two options here:

1) If interaction with sitescripts (or our redirector, or something else) needs
to be tested, then the setup just has to be more complicated. This might mean
setting up nginx locally and have it proxy the test server in addition to other
processes like the multiplexer. That's much closer to the real thing then. If
the setup gets too complicated then maybe you really need to set up a proper
infrastructure VM and import your changeset there.

2) If we *really* want to run sitescripts within the test server then it should
be a generic hook, e.g.:

  runserver.py --handler
/latest/=sitescripts.extensions.web.downloads.handle_request

Here handle_request function is supposed to be a regular WSGI app. Somebody
running the test server with that command should know to configure the handler
properly.