-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
implement performance mode for existing state size check #4392
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, seems good to me
REFLEX_PERF_MODE: EnvVar[Optional[PerformanceMode]] = env_var(PerformanceMode.WARN) | ||
|
||
# The maximum size of the reflex state in kilobytes. | ||
REFLEX_STATE_SIZE_LIMIT: EnvVar[int] = env_var(1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we set the default state size limit at the same level as the one on the WS ?
So if the State instance goes above this limit it can't be sent over the WS, would make sense to warn/raise there 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't even know about a ws size limit. I will take a look at this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lendemor where do I find the ws limit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's 1mb from socketio https://python-socketio.readthedocs.io/en/stable/api.html
max_http_buffer_size – The maximum size that is accepted for incoming messages. The default is 1,000,000 bytes. In spite of its name, the value set in this argument is enforced for HTTP long-polling and WebSocket connections.
Let's merge this as-is and continue further checks in a followup? |
@benedikt-bartscher we need a rebase for this :D |
Thanks for the heads-up, done ✔️ |
Wip #4388
Bonus: made state size warning adjustable via env. Currently, it is hardcoded to 100kb