Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#2169 closed change (fixed)

Unify development and production implementation of multiplexer

Reported by: fhd Assignee: sebastian
Priority: Unknown Milestone:
Module: Sitescripts Keywords:
Cc: sebastian, trev, matze Blocked By:
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/6209629180657664

Description (last modified by sebastian)

Background

We're currently using Flask for the sitescripts development server, and a raw WSGI app running as FastCGI via flup in production. This is problematic, because things that work fine in development (e.g. responses are automatically encoded as utf-8) don't work in production.

What to change

Unify the multiplexer implementation into a common WSGI app, used both in production and by the development server using werkzeug.serving if installed, falling back to wsgiref.simple_server.

Change History (10)

comment:1 Changed 5 years ago by fhd

  • Cc sebastian trev added

Not Ready yet, needs discussing.

I'd prefer the first option: Sure, it adds some magic we might not need. On the other hand, some of that magic is actually useful (e.g. how Flask handles utf-8 responses), and it's, how I see it, the only way that will make our development and production environment nearly identical.

comment:2 Changed 5 years ago by fhd

  • Cc matze added

comment:3 Changed 5 years ago by fhd

  • Summary changed from Use the same logic in development and production to Use a similar HTTP layer in development and production

comment:4 follow-up: Changed 5 years ago by sebastian

There is no point in using Flask the way multiplexer is currently implemented. Since URL routing as well as response handling is implemented from scratch. The reason our development server still automatically encodes unicode objects is a bug in werkzeug.serving which relies on BaseHttpServer which does that magic.

So if you want to take advantage of Flask you should replace the multiplexer, using Flask's URL routing mechanism and Response objects instead. Fine with me, as long as you adapt all scripts currently using multiplexer, and make sure they are properly tested.

Otherwise, I'd suggest to use werkzeug.serving.run_simple() directly with a wrapper raising an exception if unicode is send for our development environment.

comment:5 in reply to: ↑ 4 Changed 5 years ago by trev

Replying to sebastian:

The reason our development server still automatically encodes unicode objects is a bug in werkzeug.serving which relies on BaseHttpServer which does that magic.

That was my mistake - I was testing it with the CMS test server, yet missed that it was encoding responses elsewhere. So both werkzeug.serving and wsgiref don't encode Unicode automatically which is correct.

I dislike the idea of relying of something as complicated as Flask instead of the trivial framework currently used by sitescripts, also given the amount of unnecessary changes it will require. So my suggestion would be using werkzeug.serving.run_simple() for the test server with a fallback to wsgiref like we've done for #2196. This will bring us back to "pure WSGI" and hopefully no more compatibility issues.

Getting rid of the different host processes will be more tricky. It seems that uWSGI supports running both as a standalone HTTP and a FastCGI server (in addition to the probably more efficient uWSGI protocol). However, switching to that on our servers will likely be a lengthy process. Also, I'm not sure whether installing it is all that easy on all platforms.

comment:6 Changed 5 years ago by sebastian

  • Description modified (diff)
  • Owner set to sebastian
  • Ready set
  • Summary changed from Use a similar HTTP layer in development and production to Unify development and production implementation of multiplexer

comment:7 Changed 5 years ago by sebastian

  • Review URL(s) modified (diff)

comment:8 Changed 5 years ago by trev

  • Status changed from new to reviewing

comment:9 Changed 5 years ago by sebastian

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.