6
\$\begingroup\$

Edit: How should I interpret the silence? On a scale from 0 to 10 where 0 means "Bloody awful" and 10 means "Nothing to complain about".

I'm mainly concerned about readability and things I don't seem to be aware of. If the code is readable, I won't need to explain it.

#!/usr/bin/python

import os
import sys
import subprocess

assert __name__ != '__main__'
max_load_avg1 = 3.5

'''
compressout
===========

Simple CGI output module.
Uses gzip compression on the output stream if the client accepts it.

NOTICE: The `cgitb` module will write to stdout if the script crashes,
you should use a browser that does not accept gzip, when you are
testing your scripts.

NOTICE: In the beginning of this file `max_load_avg1` is defined.
This is the maximum allowed load average under one minute.
If the one minute load average exceeds this value, compressout will
abort.


Functions
=========

init(write_headers=True)
------------------------

    Initialize the module.  This function will detect if the client
    supports gzip.
    If `write_headers`, write a 'Vary' and (if used)
    'Content-Encdoing' header.


write_h(s)
----------

    Write part of header.
    Write `s` to standard output, will never go through gzip.


write_b(s)
----------

    Write part of body.

    gzip is supported by the client
    -------------------------------

        `s` will be appended to a local buffer
        which `done` will compress and print.

    gzip is not supported
    ---------------------

        `s` will go straight to stdout.


done()
------

    Done writing output.
    This function will invoke gzip.


Dos and don'ts
==============

## ## ##
if __name__ == '__main__':
    compressout.init()
    main()
    compressout.done()
## ## ##

    * Never call `write_h` after any call to `write_b`
    * Always call `done` when your done.
    * Use only compressout to write output
    * NOTICE: The `cgitb` module will write to stdout if the script
      crashes, you should use a browser that does not accept gzip,
      when you are testing your scripts.
'''

http503_body = '''
Service temporarily unavailable!
Wait at least two minutes before trying again.
Re-attempting prematurely may result in banning your IP address.

-- END OF TRANSMISSION --

'''

def init(write_headers=True):
    '''
    Initialize the module.  This function will detect if the client
    support gzip.
    If `write_headers`, write a 'Vary' and (if used)
    'Content-Encoding' header.
    '''
    global use_gzip
    global body

    # This is the only place where sending a 503 message will work.
    # write_h:
    #   - Message body may need to be compressed.
    #   - Possibility of conflicting Status headers.
    # write_b:
    #   - Message body may need to be compressed.
    #   - Message body may be application/xhtml+xml
    # done:
    #   - Message body needs to be compressed if `use_gzip`.
    #   - Body has already been written if not `use_gzip`.
    if os.getloadavg()[0] > max_load_avg1:
        sys.stdout.write('Status: 503\n')
        sys.stdout.write('Content-Type: text/plain\n')
        sys.stdout.write('Retry-After: 90\n')
        sys.stdout.write(http503_body)
        sys.stdout.flush()
        os.abort()

    use_gzip = 'gzip' in os.getenv('HTTP_ACCEPT_ENCODING', '')
    body = ''

    if write_headers:
        sys.stdout.write('Vary: Accept-Encoding\n')
        if use_gzip:
            sys.stdout.write('Content-Encoding: gzip\n')


def write_h(s):
    '''
    Write part of header.
    Write `s` to standard output, will never go through gzip.
    '''
    if os.getloadavg()[0] > max_load_avg1: os.abort()

    sys.stdout.write(s)


def write_b(s):
    '''
    Write part of body.

    gzip is supported by the client
    -------------------------------

        `s` will be appended to a local buffer
        which `done` will compress and print.

    gzip is not supported
    ---------------------

        `s` will go straight to stdout.
    '''
    global body

    if os.getloadavg()[0] > max_load_avg1: os.abort()

    if use_gzip:
        body += s
    else:
        sys.stdout.write(s)


def done():
    '''
    Done writing output.
    This function will invoke gzip.
    '''
    if os.getloadavg()[0] > max_load_avg1: os.abort()

    if use_gzip:
        gzip = subprocess.Popen(
            ['gzip'],
            stdin=subprocess.PIPE,
            stdout=subprocess.PIPE
        )
        sys.stdout.write(gzip.communicate(body)[0])

use_gzip = True or False
body = ''

I realize there are a few things I should have commented when I edited this module a while ago. But for "correctness" sake, I won't change the code in the question.

The current code can be found at: https://oskog97.com/read/?path=/compressout.py

\$\endgroup\$
  • \$\begingroup\$ Is there any other way than adding for spaces before every line of code? It's a pain in the ass to insert code in posts here. \$\endgroup\$ – Oskar Skog Mar 7 '17 at 14:08
  • 3
    \$\begingroup\$ The easiest way to post code is to paste it, highlight it, and press Ctrl-K to mark it as a code block. \$\endgroup\$ – 200_success Mar 7 '17 at 14:17
2
\$\begingroup\$

A few things I've noticed from a quick skim:

  • The assert is kinda useless.
  • write_h and write_b?? Documentation isn't an excuse for giving your functions useless names. Name them write_header and write_body or something.
  • Globals? Just no. Especially when they're defined AFTER the functions. Took me like 5 minutes to find them.
  • You've copied if os.getloadavg()[0] > max_load_avg1: os.abort() tons of times. Might be a good idea to export them to a function.
  • use_gzip = True or False. ?????????????????????? What ????????????????????????
\$\endgroup\$
  • \$\begingroup\$ Do you have any suggestion what I could use instead of globals? I know about classes and objects, but I'm trying to get away with "transparent" changes as I've got lots of code that uses this module. \$\endgroup\$ – Oskar Skog Jul 26 '17 at 16:09
  • \$\begingroup\$ As a temporary workaround, I've moved them up to right below the doc string and added big shouting warning labels. ... Just for the LOLs: electricstuff.co.uk/reallydangerous.gif \$\endgroup\$ – Oskar Skog Jul 26 '17 at 16:10
  • \$\begingroup\$ Using an object is probably the best solution here, but since you don't want that, just pass the body and gzip as parameters and return tuples from the functions. \$\endgroup\$ – Tom Jul 26 '17 at 16:17
  • \$\begingroup\$ Using an object would actually be easier than doing that. I'm basically using the entire module as if it was an object. \$\endgroup\$ – Oskar Skog Jul 26 '17 at 16:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.