Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1250)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 10 months ago by kzar
Modified:
3 years, 9 months ago
Visibility:
Public.

Description

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

Messages

Total messages: 6
kzar
Patch Set 1
3 years, 10 months ago (2015-09-23 13:26:29 UTC) #1
kzar
Patch Set 2 : Avoid overzealous exception catching
3 years, 10 months ago (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 ...
3 years, 9 months ago (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 ...
3 years, 9 months ago (2015-09-29 12:47:40 UTC) #4
kzar
This would be useful for me, Manvel and Ollie. We sometimes need to test how ...
3 years, 9 months ago (2015-09-29 13:18:42 UTC) #5
Wladimir Palant
3 years, 9 months ago (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
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5