Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29328527: Issue 3112 - Support use of sitescripts URL handlers from the test server (Closed)

Sept. 23, 2015, 1:24 p.m. by kzar
Sept. 29, 2015, 4:28 p.m.


Issue 3112 - Support use of sitescripts URL handlers from the test server (Depends on this change to sitescripts https://codereview.adblockplus.org/29328524/ )

Patch Set 1 #

Patch Set 2 : Avoid overzealous exception catching #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -0 lines) Patch
M README.md View 1 chunk +12 lines, -0 lines 0 comments Download
M cms/bin/test_server.py View 1 1 chunk +10 lines, -0 lines 1 comment Download


Total messages: 6
Patch Set 1
Sept. 23, 2015, 1:26 p.m. (2015-09-23 13:26:29 UTC) #1
Patch Set 2 : Avoid overzealous exception catching
Sept. 23, 2015, 1:54 p.m. (2015-09-23 13:54:55 UTC) #2
Sebastian Noack
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.py#newcode106 cms/bin/test_server.py:106: from sitescripts.web import get_handler I'm not too happy with ...
Sept. 29, 2015, 11:45 a.m. (2015-09-29 11:45:31 UTC) #3
Sebastian Noack
Dave and I discussed that approach a little more on IRC but didn't come to ...
Sept. 29, 2015, 12:47 p.m. (2015-09-29 12:47:40 UTC) #4
This would be useful for me, Manvel and Ollie. We sometimes need to test how ...
Sept. 29, 2015, 1:18 p.m. (2015-09-29 13:18:42 UTC) #5
Wladimir Palant
Sept. 29, 2015, 4:21 p.m. (2015-09-29 16:21:21 UTC) #6
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

* 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

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

Powered by Google App Engine
This is Rietveld