fix(config): preserve preloaded rxconfig classes - #7144
FarhanAliRaza wants to merge 6 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe952bca61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…-boundary # Conflicts: # reflex/utils/exec.py # tests/units/utils/test_exec.py
masenf
left a comment
masenf
left a comment
There was a problem hiding this comment.
does this break the mode where i invoke granian directly against my reflex app module? it seems i would need to change my invocation to granian --interface asgi --factory reflex.utils.exec:load_app.
this moves us farther away from the goal of asgi interoperability, because now a special shim is required to start the app.
is it not sufficient to just get the config inside App.__post_init__? then it could be transparently used by any asgi server
…backend factory shim The first get_config() load of a context re-executed rxconfig.py even when the app module had already imported it, so a direct ASGI invocation still got a second copy of every class. Reuse the imported module for that first load and drop the load_app factory; exec.py is back to its main version.
| with _load_config_lock: | ||
| if ctx._config is None: | ||
| ctx._set_config(_get_config()) | ||
| ctx._set_config(_imported_config(Path.cwd().resolve()) or _get_config()) |
There was a problem hiding this comment.
When project code has already imported rxconfig, a forked RegistrationContext receives that module's existing mutable Config object. This conflicts with RegistrationContext.fork(), which resets _config and promises that the next get_config() call reloads it. The parent and fork can therefore share configuration mutations, allowing persistent overrides or other changes from one app to leak into the other.
Knowledge Base Used: Application lifecycle and configuration
2 participants
Summary
Classes defined in
rxconfig.py(for example arx.Statesubclass the app imports from its config module) lost their identity because the config module was executed more than once per process. The app module importsrxconfig, thenApp.__init__calledreload_config(), which re-executedrxconfig.pyand produced a second copy of every class it defines. The result was aStateValueErrorfor the duplicate substate, or_statepointing at a class the app never imported, and pickle round-trips that failed to resolve the original type.The fix lives in the config loader and in
App, so it applies to any ASGI server. No special backend entry point is needed;granian --interface asgi --factory myapp.myapp:appworks unchanged.App.__init__now callsget_config()instead ofreload_config(), reusing the config already loaded in the activeRegistrationContext.get_config()load of a context reuses anrxconfigmodule that project code already imported from the project root, instead of executing the file again. A module that_get_config()itself loaded is not reused, so separate contexts still get independentConfiginstances.reload_config()still always loads from disk.reflex rundrops its redundantreload_config()after persisting the host and port overrides; those values are already written through_set_persistent.Configslot on the context between tests as well as theAppslot, so a mocked config from one test cannot leak into the next.reflex/utils/exec.pyis unchanged frommain.Tests
tests/units/test_app.py: classes defined inrxconfig.pykeep their identity acrossApp()construction, both when the config was loaded first and when the app module importedrxconfigfirst (the direct ASGI case).Validation
rxconfig.pydefines anrx.State: directgranian --factory pkg.mod:appfailed withStateValueErrorbefore and serves requests now;reflex rundev startup, State events, and hot reload of the app file and ofrxconfig.pywork.