airavata-courses / SuperNova

Spring 2022 Project
Apache License 2.0
0 stars 0 forks source link

Project Review #105

Closed amolsangar closed 2 years ago

amolsangar commented 2 years ago

Everything looks good although I found one quick improvement area.

The weather API service is addressing requests one after another rather than handling them simultaneously. The requests are getting blocked until all of them are processed.

Using Async doesn't always give the desired results.

Test - sending 2 simultaneous requests getting processed one after another image

Proposed solution - The default multithreading of Fast API can be used to tackle this problem. Replace the async function by normal (def) function which enables multithreading to handle requests otherwise disabled for async functions.

Replace this - @app.get("/weatherApi/plot") async def read_root(radar_id, date):

by this - @app.get("/weatherApi/plot") def read_root(radar_id, date):

Requests getting simultaneously processed. image

Reference - https://medium.com/@rishibajargan/do-not-over-use-async-with-fastapi-fa70bed14d9c

Hope this helps!

- Team Puzzles

sanket1211996 commented 2 years ago

@amolsangar Really Appreciate the feedback! Detailed explanation help us to understand the root cause. I was doubtful regarding the default multithreading and async call. You point it correctly even if async in place its not blocking the requests but just processing it one after another. We will take care of this in upcoming release work. Point Noted.

SreeshaKS commented 2 years ago

Good work on the peer review!