Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support http.HTTPStatus #1135

Closed
floqqi opened this issue Nov 8, 2017 · 9 comments · Fixed by #1735
Closed

Support http.HTTPStatus #1135

floqqi opened this issue Nov 8, 2017 · 9 comments · Fixed by #1735
Assignees
Milestone

Comments

@floqqi
Copy link

floqqi commented Nov 8, 2017

It would be great to support http.HTTPStatus from the standard library (3.5+).

Could possibly look like:

from http import HTTPStatus

class MyResource:
    def on_get(self, req, res):
        res.status = HTTPStatus.NOT_FOUND  # instead of falcon.HTTP_404
@kgriffs kgriffs added this to the Version 2.0 milestone Nov 29, 2017
@kgriffs
Copy link
Member

kgriffs commented Nov 29, 2017

Not a bad idea. Since 1.4 is on the home stretch, let's look at this for 2.0. For the sake of backwards-compatibility, we will probably want to allow both the enum and the raw string.

@vytas7
Copy link
Member

vytas7 commented Jan 2, 2018

As discussed on Gitter, I can take a stab at this.

Basic design:

  • Python 3.5+ http.HTTPStatus or its subclass in Falcon will be THE way to set status
  • Python 3.5+ http.HTTPStatus will be backported to be used in older Python versions
  • Falcon 1.x status constants falcon.HTTP_* will be aliased to the new enum values to maintain backward compatibility for the most common usecases

Note that, strictly speaking, it will still be a breaking change in type compatibility, so setting status to raw strings not obtained from falcon.HTTP_* constants will yield a type error.

Open question: support for HTCPCP/1.0 status code HTCPCP 418 I'm a teapot. It is present in the current Falcon constants, but absent in http.HTTPStatus. Time permitting a shim could be implemented around http.HTTPStatus, otherwise the work might be deferred until the 1st of April.

@vytas7
Copy link
Member

vytas7 commented Jan 16, 2018

@kgriffs on Gitter, we agreed that a lookup table to speed things up is a reasonable approach to proceed.

You also indirectly implied it could be a status property:

Kurt Griffiths
Agreed, it should be good enough. One area where Falcon is opinionated is encouraging apps to follow the RFCs. Inventing app-specific codes sort of violates the uniform interface.
In a pinch, I suppose someone could subclass Request (probably Response // Vytas) and override the status prop

Another alternative I had in my mind was having a lookup table on the API side.
The drawback is that should one desire to use some completely nonstandard status line (yes, we are going to maintain 7XX, teapots and coffee machines), it becomes even harder and dirtier to hook that in. Unless we add an API for that.

Thoughts?

In test.py (see pasted below), a lookup table on the API side (hence no use of properties) seems to outdo even the current approach (at least on py27) due to the absence of if six.PY2 clause.

$ python2 test.py bench
1.17599201202
1.28570485115
1.02982211113

$ python2 test.py bench
1.18289780617
1.27076292038
1.05143809319

$ python2 test.py bench
1.18374109268
1.28530693054
1.10190296173
$ python3 test.py bench
1.199260131000301
1.3444200409999212
1.2164098600001125

$ python3 test.py bench
1.2904272340001626
1.3780863749998389
1.2642014110001583

$ python3 test.py bench
1.3386266430002252
1.4676438950000374
1.3318281369997749

test.py reads:

from wsgiref.simple_server import make_server
from wsgiref.validate import validator
import enum  # needs enum34 on Python < 3.5
import sys
import timeit

import six

try:
    from http import HTTPStatus
except ImportError:
    HTTPStatus = None


# note: Python library code below does not pass PEP8 checks ;)
class BackportedHTTPStatus(enum.IntEnum):
    """HTTP status codes and reason phrases

    Status codes from the following RFCs are all observed:

        * RFC 7231: Hypertext Transfer Protocol (HTTP/1.1), obsoletes 2616
        * RFC 6585: Additional HTTP Status Codes
        * RFC 3229: Delta encoding in HTTP
        * RFC 4918: HTTP Extensions for WebDAV, obsoletes 2518
        * RFC 5842: Binding Extensions to WebDAV
        * RFC 7238: Permanent Redirect
        * RFC 2295: Transparent Content Negotiation in HTTP
        * RFC 2774: An HTTP Extension Framework
    """
    def __new__(cls, value, phrase, description=''):
        obj = int.__new__(cls, value)
        obj._value_ = value

        obj.phrase = phrase
        obj.description = description
        return obj

    # informational
    CONTINUE = 100, 'Continue', 'Request received, please continue'
    SWITCHING_PROTOCOLS = (101, 'Switching Protocols',
            'Switching to new protocol; obey Upgrade header')
    PROCESSING = 102, 'Processing'

    # success
    OK = 200, 'OK', 'Request fulfilled, document follows'
    CREATED = 201, 'Created', 'Document created, URL follows'
    ACCEPTED = (202, 'Accepted',
        'Request accepted, processing continues off-line')
    NON_AUTHORITATIVE_INFORMATION = (203,
        'Non-Authoritative Information', 'Request fulfilled from cache')
    NO_CONTENT = 204, 'No Content', 'Request fulfilled, nothing follows'
    RESET_CONTENT = 205, 'Reset Content', 'Clear input form for further input'
    PARTIAL_CONTENT = 206, 'Partial Content', 'Partial content follows'
    MULTI_STATUS = 207, 'Multi-Status'
    ALREADY_REPORTED = 208, 'Already Reported'
    IM_USED = 226, 'IM Used'

    # redirection
    MULTIPLE_CHOICES = (300, 'Multiple Choices',
        'Object has several resources -- see URI list')
    MOVED_PERMANENTLY = (301, 'Moved Permanently',
        'Object moved permanently -- see URI list')
    FOUND = 302, 'Found', 'Object moved temporarily -- see URI list'
    SEE_OTHER = 303, 'See Other', 'Object moved -- see Method and URL list'
    NOT_MODIFIED = (304, 'Not Modified',
        'Document has not changed since given time')
    USE_PROXY = (305, 'Use Proxy',
        'You must use proxy specified in Location to access this resource')
    TEMPORARY_REDIRECT = (307, 'Temporary Redirect',
        'Object moved temporarily -- see URI list')
    PERMANENT_REDIRECT = (308, 'Permanent Redirect',
        'Object moved temporarily -- see URI list')

    # client error
    BAD_REQUEST = (400, 'Bad Request',
        'Bad request syntax or unsupported method')
    UNAUTHORIZED = (401, 'Unauthorized',
        'No permission -- see authorization schemes')
    PAYMENT_REQUIRED = (402, 'Payment Required',
        'No payment -- see charging schemes')
    FORBIDDEN = (403, 'Forbidden',
        'Request forbidden -- authorization will not help')
    NOT_FOUND = (404, 'Not Found',
        'Nothing matches the given URI')
    METHOD_NOT_ALLOWED = (405, 'Method Not Allowed',
        'Specified method is invalid for this resource')
    NOT_ACCEPTABLE = (406, 'Not Acceptable',
        'URI not available in preferred format')
    PROXY_AUTHENTICATION_REQUIRED = (407,
        'Proxy Authentication Required',
        'You must authenticate with this proxy before proceeding')
    REQUEST_TIMEOUT = (408, 'Request Timeout',
        'Request timed out; try again later')
    CONFLICT = 409, 'Conflict', 'Request conflict'
    GONE = (410, 'Gone',
        'URI no longer exists and has been permanently removed')
    LENGTH_REQUIRED = (411, 'Length Required',
        'Client must specify Content-Length')
    PRECONDITION_FAILED = (412, 'Precondition Failed',
        'Precondition in headers is false')
    REQUEST_ENTITY_TOO_LARGE = (413, 'Request Entity Too Large',
        'Entity is too large')
    REQUEST_URI_TOO_LONG = (414, 'Request-URI Too Long',
        'URI is too long')
    UNSUPPORTED_MEDIA_TYPE = (415, 'Unsupported Media Type',
        'Entity body in unsupported format')
    REQUESTED_RANGE_NOT_SATISFIABLE = (416,
        'Requested Range Not Satisfiable',
        'Cannot satisfy request range')
    EXPECTATION_FAILED = (417, 'Expectation Failed',
        'Expect condition could not be satisfied')
    UNPROCESSABLE_ENTITY = 422, 'Unprocessable Entity'
    LOCKED = 423, 'Locked'
    FAILED_DEPENDENCY = 424, 'Failed Dependency'
    UPGRADE_REQUIRED = 426, 'Upgrade Required'
    PRECONDITION_REQUIRED = (428, 'Precondition Required',
        'The origin server requires the request to be conditional')
    TOO_MANY_REQUESTS = (429, 'Too Many Requests',
        'The user has sent too many requests in '
        'a given amount of time ("rate limiting")')
    REQUEST_HEADER_FIELDS_TOO_LARGE = (431,
        'Request Header Fields Too Large',
        'The server is unwilling to process the request because its header '
        'fields are too large')

    # server errors
    INTERNAL_SERVER_ERROR = (500, 'Internal Server Error',
        'Server got itself in trouble')
    NOT_IMPLEMENTED = (501, 'Not Implemented',
        'Server does not support this operation')
    BAD_GATEWAY = (502, 'Bad Gateway',
        'Invalid responses from another server/proxy')
    SERVICE_UNAVAILABLE = (503, 'Service Unavailable',
        'The server cannot process the request due to a high load')
    GATEWAY_TIMEOUT = (504, 'Gateway Timeout',
        'The gateway server did not receive a timely response')
    HTTP_VERSION_NOT_SUPPORTED = (505, 'HTTP Version Not Supported',
        'Cannot fulfill request')
    VARIANT_ALSO_NEGOTIATES = 506, 'Variant Also Negotiates'
    INSUFFICIENT_STORAGE = 507, 'Insufficient Storage'
    LOOP_DETECTED = 508, 'Loop Detected'
    NOT_EXTENDED = 510, 'Not Extended'
    NETWORK_AUTHENTICATION_REQUIRED = (511,
        'Network Authentication Required',
        'The client needs to authenticate to gain network access')


HTTPStatus = HTTPStatus or BackportedHTTPStatus

HTTP_TOO_MANY_REQUESTS_0 = '429 Too Many Requests'
HTTP_TOO_MANY_REQUESTS_1 = HTTPStatus(429)


class BareResponse0(object):

    def __init__(self):
        self.status = None


class BareResponse1(object):

    _LOOKUP_TABLE = {status: "{} {}".format(status, status.phrase)
                     for status in HTTPStatus}

    def __init__(self):
        self._raw_status = None

    @property
    def status(self):
        return self._raw_status

    @status.setter
    def status(self, value):
        self._raw_status = self._LOOKUP_TABLE[value]


class BareMetalAPI0(object):

    def __init__(self):
        pass

    def wsgi(self, environ, start_response):
        response = BareResponse0()
        response.status = HTTP_TOO_MANY_REQUESTS_0

        status = str(response.status) if six.PY2 else response.status
        headers = [('Content-type', 'text/plain')]
        start_response(status, headers)
        return ['Too fast!']


class BareMetalAPI1(object):

    def __init__(self):
        pass

    def wsgi(self, environ, start_response):
        response = BareResponse1()
        response.status = HTTP_TOO_MANY_REQUESTS_1

        status = response._raw_status
        headers = [('Content-type', 'text/plain')]
        start_response(status, headers)
        return ['Too fast!']


class BareMetalAPI2(object):

    def __init__(self):
        self._LOOKUP_TABLE = {status: "{} {}".format(status, status.phrase)
                              for status in HTTPStatus}

    def wsgi(self, environ, start_response):
        response = BareResponse0()
        response.status = HTTP_TOO_MANY_REQUESTS_1

        status = self._LOOKUP_TABLE[response.status]
        headers = [('Content-type', 'text/plain')]
        start_response(status, headers)
        return ['Too fast!']


api0 = BareMetalAPI0().wsgi
api1 = BareMetalAPI1().wsgi
api2 = BareMetalAPI2().wsgi


def test0():
    def start_response(status, headers):
        assert status == '429 Too Many Requests'
    api0({}, start_response)


def test1():
    def start_response(status, headers):
        assert status == '429 Too Many Requests'
    api1({}, start_response)


def test2():
    def start_response(status, headers):
        assert status == '429 Too Many Requests'
    api2({}, start_response)


if __name__ == '__main__':
    # Exercise for the reader: use argparse or similar
    assert len(sys.argv) == 2, "{} bench/run".format(sys.argv[0])
    command = sys.argv[1]
    assert command in ('bench', 'run'), "{} bench/run".format(sys.argv[0])

    if command == 'bench':
        print(timeit.timeit(test0))
        print(timeit.timeit(test1))
        print(timeit.timeit(test2))
    else:
        validator_app = validator(api2)
        httpd = make_server('', 8000, validator_app)
        print('Listening on port 8000...')
        httpd.serve_forever()

@xgfone
Copy link

xgfone commented May 16, 2018

A good idea!

When using falcon, I like int as the status. But falcon does not support it, so I wrap it.

import falcon

STATUS_CODES = {}
for v in vars(falcon.status_codes).values():
    if isinstance(v, str):
        try:
            code = int(v.split(maxsplit=1)[0])
            STATUS_CODES[code] = v
        except Exception:
            pass

class Resource(object):
    def status(self, code):
        return STATUS_CODES[code]

    def send_response(self, resp, status=200, body=None, content_type=None):
        resp.status = self.status(status)
        if content_type:
            resp.content_type = content_type
        if body:
            resp.body = body

   def on_get(self, req, resp):
        self.send_response(resp)

But, it's tedious. If falcon supports it natively, it's perfect!

@vytas7
Copy link
Member

vytas7 commented Jun 30, 2018

Resuming work on this!

@kgriffs
Copy link
Member

kgriffs commented Nov 4, 2019

An alternative approach to this that I think has worked out pretty well on the ASGI side:

Use the new falcon.util.misc.http_status_to_code() method from #1573 to convert the value so that it can be used in the response. It uses functools.lru_cache to make it blazing fast.

We'll also need to update the docs to let users know that they can now set resp.status to anything that http_status_to_code() accepts (we may want to update the tutorials/examples as well to use instances of http.HTTPStatus from the standard library).

@vytas7
Copy link
Member

vytas7 commented Nov 4, 2019

I'm assigning myself for the time being, if anyone wants to help out, feel free to volunteer!
Otherwise I'll try to squeeze this in for the 3.0 release myself.

@vytas7 vytas7 removed their assignment Nov 4, 2019
@vytas7 vytas7 added needs contributor Comment on this issue if you'd like to volunteer to work on this. Thanks! and removed in progress labels Nov 4, 2019
@vytas7
Copy link
Member

vytas7 commented Nov 6, 2019

@kgriffs have you read upon functools.lru_cache thread safety issues btw?

As I understand the problem is fixed on the newest versions of CPython, and probably backported even to down to 3.5? Might it still be an issue if popular distributions still ship the unpatched variant?

FWIW the default Python 3 on my Ubuntu 16.04 workstation is 3.5.2, and I could unfortunately reproduce the issue immediately with one of the attached test cases:

import functools
import threading
import time


@functools.lru_cache(maxsize=2)
def f(v):
    time.sleep(.01)


threads = [threading.Thread(target=f, args=(v,)) for v in [1,2,2,3,2]]
for t in threads:
    t.start()
Exception in thread Thread-5:
Traceback (most recent call last):
  File "/usr/lib/python3.5/threading.py", line 914, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.5/threading.py", line 862, in run
    self._target(*self._args, **self._kwargs)
KeyError: (2,)

16.04 is still pretty ubiquitous in the server world, I think even our Travis uses that (although Travis copies over newest Python tarballs).

@kgriffs
Copy link
Member

kgriffs commented Dec 17, 2019

Ugh, that's awful. Based on some quick tests, it looks like it is fixed as of 3.5.4+, 3.6.1+, 3.7.0+ and 3.8.0. I suppose we could detect the version and then swap out the decorator for a NOP function as needed?

@vytas7 vytas7 added in progress and removed needs contributor Comment on this issue if you'd like to volunteer to work on this. Thanks! labels Jul 4, 2020
@vytas7 vytas7 self-assigned this Jul 4, 2020
vytas7 added a commit to vytas7/falcon that referenced this issue Jul 4, 2020
vytas7 added a commit that referenced this issue Jul 31, 2020
…1735)

* feat(Response): support setting Response.status to http.HTTPStatus

* docs(newsfragments): fix a function link for #1135

Co-authored-by: Kurt Griffiths <mail@kgriffs.com>
@vytas7 vytas7 removed the in progress label Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants