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

Rewrite search API to use fastapi, aioredis-py #3

Merged
merged 19 commits into from
May 19, 2021
Merged

Rewrite search API to use fastapi, aioredis-py #3

merged 19 commits into from
May 19, 2021

Conversation

abrookins
Copy link
Contributor

@abrookins abrookins commented May 7, 2021

Rewrite the search API to use fastapi and aioredis-py.

Current benchmarks:

FastAPI

$ wrk -c 60 -t 20 "https://stage-search-service.redislabs.com/search?q=python"
Running 10s test @ https://stage-search-service.redislabs.com/search?q=python
  20 threads and 60 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    70.33ms   52.08ms 596.22ms   93.01%
    Req/Sec    46.60     13.90    90.00     74.94%
  9180 requests in 10.10s, 51.01MB read
Requests/sec:    908.78
Transfer/sec:      5.05MB

Falcon

$ wrk -c 60 -t 20 "https://search-service.redislabs.com/search?q=python" 
Running 10s test @ https://search-service.redislabs.com/search?q=python
  20 threads and 60 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   214.01ms   71.38ms 493.73ms   70.17%
    Req/Sec    14.21      6.56    40.00     88.77%
  2768 requests in 10.09s, 15.93MB read
Requests/sec:    274.22
Transfer/sec:      1.58MB

@abrookins abrookins marked this pull request as ready for review May 19, 2021 21:13
@abrookins abrookins merged commit 080aed0 into master May 19, 2021
Copy link

@joeriddles joeriddles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, coming from your blog post about FastAPI 👋

The FastAPI refactor looks pretty good to me. I agree that FastAPI's way of authentication leaves a bit to be desired.

docs = transform_documents(docs, search_site, q.query_string())
resp.body = json.dumps({"total": total, "results": docs})
from_url = from_url if from_url else ''
start = start if isinstance(start, int) else 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is isinstance necessary here, since start has a type hint of Optional[int]? FastAPI uses Pydantic under the hood, which will attempt to coerce anything passed-in to the function to an int.

pydantic uses int(v) to coerce types to an int; see this warning on loss of information during data conversion

https://pydantic-docs.helpmanual.io/usage/types/

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.

2 participants