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

[ENG-4326] Async ComputedVar #4711

Merged
merged 15 commits into from
Feb 1, 2025
Merged

[ENG-4326] Async ComputedVar #4711

merged 15 commits into from
Feb 1, 2025

Conversation

masenf
Copy link
Collaborator

@masenf masenf commented Jan 29, 2025

  • rx.var can now decorate an async function (Accessing the var on the instance requires await)
  • Move computed var dependency tracking to a separate module
  • States can have _var_dependencies on other states
  • Rewrite StateManagerRedis get_state routine to accomodate fetching of arbitrary state dependencies
    • Fetch all required states in one redis pipeline, then link the states up later. This is more efficient and flexible than the previous recursive get_state method.
  • Expose .add_dependency on ComputedVar to allow compile time dynamic dependency specification

Fixes #4022

Copy link

linear bot commented Jan 29, 2025

these might be added dynamically later in which case we recompute the
dependency tracking dicts... if not, they'll blow up anyway at runtime.
@benedikt-bartscher
Copy link
Contributor

This is awesome @masenf, I will definitely try it out as soon as possible!

btw, Closes #4022

except RuntimeError:
pass
else:
if is_in_app_harness():
Copy link
Member

Choose a reason for hiding this comment

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

is it possible to abstract this logic beyond being specific to app harness? is it possible to detect that an event loop running?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's possible to detect it (which is what we're doing just above), but that would be abnormal conditions for a reflex app outside of a testing environment, so maybe we want to fail on it?

Copy link
Member

Choose a reason for hiding this comment

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

an error sounds fine then, if someone complains we will learn something 🤓

Copy link

codspeed-hq bot commented Jan 31, 2025

CodSpeed Performance Report

Merging #4711 will not alter performance

Comparing masenf/async-computed-var (c7fc787) with main (8663dbc)

Summary

✅ 1 untouched benchmarks

@masenf masenf merged commit a224319 into main Feb 1, 2025
39 checks passed
@masenf masenf deleted the masenf/async-computed-var branch February 1, 2025 00:33
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.

[Enhancement] Async cached_var in the state
3 participants