forked from kytos/kytos
-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: replaced werkzeug/flask
with uvicorn/starlette
#375
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
removed flask, flask-socketio, flask_cors
api_client dead_letter auth Deleted autouse ev_loop fixture to avoid ev loop conflicts
Replaced werkzeug with uvicorn Adapted APIServer methods accordingly Used httpx when fetching ui web latest release tag
Refactored DeadLetter endpoints to be async
Introduced to validate async routes Broken down functions for reusability
Changed initial log level as INFO for uvicorn qualname
This was referenced Apr 24, 2023
a89b6eb
to
0b63eff
Compare
…be deterministic
Parametrized ev loop on Controller to be used with get_json funcs Refactored accordingly
This reverts commit 3eec164.
Removed flask optional dependency from elastic-apm
This was referenced May 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #347
Closes #372
Closes #301
Closes #168
Closes #280
Closes #225
Summary
See updated changelog file.
Summary of upcoming
starlette/uvicorn
changes for NApp developers to be aware:async
client,get_test_client
, will return an instance ofhttpx.asyncclient
. Although it's anasync
client it can test both sync or async client, but the test case must be async, this is a constraint to ensure that pytest and event loops plays well (as mentioned earlier,unittest.testcase
isn't compatible).async
endpoints over sync endpoints for IO-bound work. if you're usingpymongo
or any other lib that's blocking you should stick with sync endpoints to avoid blocking the event loop, sincestarlette
will run sync endpoints in a threadpool. notice that race conditions can still happen, but they're easier to manage since context switching is explicit, andthreading.lock
isn't compatible, so if you have a dependency using athreading.lock
and can't be migrated or moved then you should stick with a sync endpoint too.starlette
withuvicorn
generally outperformsflask
anduvicorn
in most cases, latency has been also improved even for cases where sync endpoints are used, since uvicorn threadpool machinery is a bit more optimized.kytos
core dependencies will shiphttpx
, it's both a sync andasync
version ofrequests
, relatively the same usability. you don't need to replace existingrequests
usage on our NApps but http calls should preferably usehttpx
since by default it works synchronously but is compatible withasyncio
too.uvicorn
supports auto reload, but auto reloading the entire process isn't trivial especially considering the foreground mode and also how NApps start/stop and their life cycle,uvicorn
provides a slightly better experience when serving the ui files, so even though we won't have yet a full blown hot-reload for any un changes, the workflow when developing ui for a Napp won't have that much friction though, since a page refresh is expected to work with any new changes on.kytos
files.starlette
unlocks python-based websockets implementation, in the future, we could also allow websocket routes for bidirectional communications for certain NApps in the future.pymongo
, one day we might also introduce anasync
option withmotor
, but apm instrumentation doesn't work with it yet, and its implementation is just wrappingasyncio
over threads (it could still be handy, but it's worth waiting to see how it'll evolve). other than that, you should be able to reach out toasyncio
andasyncio
-compatible libs to pretty much any other io parts of our code base.Benchmark
Here's the benchmark request stress tests with
uvicorn
(with all of the recent draft PRs) andwerkzeug
. In summary,uvicorn
is outperforming in most cases, and overall has lower latencies metrics for bothasync
and ThreadPool-based (sync) routes:GET topology/v3
500 reqs/sec over 60 secs withuvicorn
, sync route:GET topology/v3
500 reqs/sec over 60 secs withwerkzeug
, sync route (this was the case where werkzeug lead to unstability):POST of_lldp/v1/polling_time
200 reqs/sec over 60 scs withuvicorn
,async
route (this endpoint doesn't have additional IO though to make the diff even more evident):POST of_lldp/v1/polling_time
200 reqs/sec over 60 scs withwerkzeug
, sync route (latency metrics are worse as expected withwerkzeug
compared touvicorn
):POST kytos/flow_manager/v2/flows/{dpid}
200 reqs/sec over 60 secs withuvicorn
, sync route (despitepymongo
driver is still IO-blocking, it performed better, especially comparing the mean and under 50th percentile):POST kytos/flow_manager/v2/flows/{dpid}
200 reqs/sec over 60 secs withwerkzeug
:Local Tests
I ran local tests with with all linked PRs (check out their PR summary for more info)
End-to-End Tests
e2e tests with this PR and related
starlette
PRs can be found here, they're passing