-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Move js-client bootstrapping to FastAPI #21264
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
base: dev
Are you sure you want to change the base?
Conversation
| @router.cbv | ||
| class FastAPIContext: | ||
| @router.get("/api/context", summary="Return bootstrapped client context") | ||
| def index(self, request: Request, trans: ProvidesUserContext = DependsOnTrans) -> JSONResponse: |
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.
Design-wise I think it would be a good idea to make this as static as possible so we can heavily cache the response. fetch all the user-specific stuff later ?
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.
Agreed, this is an initial very rough draft. We should remove unnecessary parts and keep it as minimal as possible. It may eventually fit into the configuration API, but it’s too early to decide.
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.
@mvdbeek The PR is in decent shape now. Splitting the user and config pieces out, or adding caching, still makes sense, but we can handle that cleanly in a follow-up once these structural changes are in dev.
4bf45df to
6221dbe
Compare
569fd5e to
6f63b93
Compare
f2c05ab to
0791c4e
Compare
db18ffa to
9ca530c
Compare
a0eac34 to
6c3d357
Compare
| root: str | ||
| config: dict[str, Any] | ||
| session_csrf_token: Optional[str] = None | ||
| user: dict[str, Any] |
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.
There is a DetailedUserModel in lib/galaxy/schema/schema.py maybe you can use that to improve the API typing here.
Of course, you will need to add the new fields for the detailed view introduced in this PR in the model too :)
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.
Switched to the DetailedUserModel.
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.
@davelopez Switching to DetailedUserModel as in 52e8671 produces double encoding of the user id because of the serialization at: https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/managers/users.py#L688. The user manager is used in several places, so removing encoding there is not guaranteed to be safe and would need broader review. This might fit better in the follow-up where config and user data move into their Pinia stores and the Galaxy.config and Galaxy.user attributes are likely to be removed.
52e8671 to
6c3d357
Compare
Requires #17507. Resolves #21153.
This replaces the entire historical bootstrap path with a single, explicit initialization flow.
The old layers disappear in full: Mako injected config, ad-hoc script tags, incremental config.set merges, RxJS config debouncing, init queues, defaultAppFactory, standardInit, and the monitor plumbing. None of these participate anymore.
Startup is reduced to one request and one construction step.
App Construction
The client issues a single GET to
/api/context. That response contains the startup payload:root, fullconfig, detaileduser, and a sessionCSRF token. No templates insert state. No merging or patching occurs afterward.initGalaxyInstancetakes the JSON, builds aGalaxyApp, registers it, and installs thewindow.Galaxygetter. Construction is uniform across all entry points.GalaxyAppoptions default deterministically. User and locale initialization no longer depend on template timing. No late mutation of config or user attributes remains. All states derive from the context payload and stay stable.Elimination of Legacy Mechanism
The template based bootstrapping path is removed.
JSAppLauncheris gone. Templates no longer try to prepare configuration or user objects. The JS bundle loads without server side state injection.All
RxJSsequencing is removed. There is no config observable, no debounce cycle, and no init queue. Nothing accumulates late additions. Initialization is synchronous except for the initial fetch.New Mount Sequence
The mount sequence becomes linear and inspectable:
windowload listener triggers.initGalaxyInstancefetches/api/contextand constructsGalaxyApp.Routeris created with access toGalaxyAppobject.SentryandWebhooksare initialized withconfiganduseravailable.No branching. No timing sensitivity. No template inserted mutations.
See: https://github.com/guerler/galaxy/blob/00fdf2556a473eb22ab16f91221c11c797b4ab2b/client/src/entry/analysis/index.ts
Configuration
All previous template delivered or JS injected configuration now arrives through
/api/context: dynamic tool confs, activation and inactivity flags, webhooks enablement, analytics DSNs, workflow menu entries, and admin extended config. Consumers read fromGalaxy.configandGalaxy.useror the Pinia stores, instead of inspecting template variables or legacy global state.Result
Client startup collapses to a predictable, single source of truth. Every subsystem sees the same configuration and the same user payload. All historical timing and merging behavior is removed. The only contract the reviewer needs to understand is the
/api/contextresponse and the fixed initialization sequence.Outlook
The context endpoint also fixes the mismatch between the initial bootstrap response and what the config and user Pinia stores expect. It already returns the exact fields produced by the regular config and user API endpoints, so the client gets the final, ready to use values on first load. That will let us fill both stores immediately without extra mapping or translation. If we later decide to fetch config and user separately, or to load parts of them on demand, nothing needs to change on the client because the context response is already built from those same endpoints. This keeps bootstrapping unified now while leaving room to break it apart cleanly later.
How to test the changes?
(Select all options that apply)
License