Skip to content

fix: make the rate limit example runnable - #2

Open
iklobato wants to merge 1 commit into
Iaguiz:masterfrom
iklobato:fix/rate-limit-example
Open

iklobato wants to merge 1 commit into
Iaguiz:masterfrom
iklobato:fix/rate-limit-example

Conversation

@iklobato

Copy link
Copy Markdown

Small fix on top of your branch for iklobato#48. Only touches examples/19_rate_limit.py.

I tried to run the example and it did not start. Three problems, each one enough to break it:

  1. from lightapi import ... LightAPI -> the class is LightApi, so the import fails right away:
ImportError: cannot import name 'LightAPI' from 'lightapi'. Did you mean: 'LightApi'?
  1. rate_limit = {"request": 3, ...} -> the middleware reads requests, so every call to /books returns 500:
GET /books HTTP/1.1" 500 Internal Server Error
KeyError: 'requests'
  1. app.run(..., reload=True) -> uvicorn needs an import string for reload, so it just prints a warning and exits:
WARNING:  You must pass the application as an import string to enable 'reload' or 'workers'.

Changed it to debug=True, which is what the other 18 examples use.

I ran the example after the change and called the endpoint:

req 1: HTTP/1.1 200 OK | x-ratelimit-remaining: 2
req 2: HTTP/1.1 200 OK | x-ratelimit-remaining: 1
req 3: HTTP/1.1 200 OK | x-ratelimit-remaining: 0
req 4: HTTP/1.1 429 Too Many Requests | x-ratelimit-remaining: 0

The middleware itself is unchanged. Your 4 tests still pass and so does the full suite (331 passed).

The example could not start. Three problems, each fatal on its own:

- `LightAPI` is not exported; the class is `LightApi`, so the import
  raised ImportError before anything ran.
- `Meta.rate_limit` used the key `request`; the middleware reads
  `requests`, so every call to /books returned 500 with KeyError.
- `run(reload=True)` needs an import string, so uvicorn refused to
  start and only printed a warning. Every other example passes
  `debug=True`.

Verified by running the example and calling the endpoint: the first
three requests return 200 with a decreasing X-RateLimit-Remaining and
the fourth returns 429.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant