-
Notifications
You must be signed in to change notification settings - Fork 12
AEP 010: A dedicated Graphical User Interface for AiiDA #45
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: master
Are you sure you want to change the base?
Conversation
010_aiida_gui/readme.md
Outdated
|
|
||
| ### Backend: FastAPI & aiida-restapi | ||
|
|
||
| * **Leverage** the existing [aiida-restapi](https://github.com/aiidateam/aiida-restapi) endpoints wherever possible. |
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.
"wherever possible" seems a big goal but not very tangible for a proposal. Can you list all endpoints you want to support in this implementation. "This implementation" I mean we can approve and make quite decision without extra functionalities.
If I remember correctly, @giovannipizzi was asking (correct me if I am wrong @giovannipizzi) not adding more like "plugin system" for the aiida-gui, but just what it already has and can make current AiiDA using the tool.
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.
All the endpoints related to the Core Views, in principle, should from the aiida-resetapi.
/processes
/nodes
/daemon
/computers
/groups
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.
So you want to support all endpoints in the GUI, or subset of those? If it is a subset, list them out.
unkcpz
left a comment
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.
thanks @superstar54, I marked some text as "out of scope" because I believe those are points that can cause more discussions. Besides that can you elaborate on the left parts on the details I asked for?
| * Commands: `aiida-gui start|stop|restart|status` | ||
| * Mount the React frontend into FastAPI so only a single process is needed | ||
|
|
||
| * **Security & Multi-User Support** |
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.
out of scope.
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.
- Token-based access (
?token=…) akin to Jupyter notebooks
This is suggested by @giovannipizzi , and I also think we need to support, thus avoid different users from the same computer has access to other user's data.
- Optional integration with JupyterHub deployment for centralized authentication
As it said, this is "Optional", and a nice feature, e.g., in the aiida tutorial, if we use AiiDAlab depoyment, the user can use this feature.
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.
thus avoid different users from the same computer has access to other user's data.
Token is not assigned by user, so this is not true statement.
As it said, this is "Optional", and a nice feature, e.g., in the aiida tutorial, if we use AiiDAlab depoyment, the user can use this feature.
Then you want to add it in the implementation or not? In the AEP, just list things you will execute in the implementation.
010_aiida_gui/readme.md
Outdated
| * Load frontend dynamically into sidebar, new pages or data viewers. | ||
| * Shared React component library (`aiida-react-components-base`) published to npm | ||
|
|
||
| * **Apps Page** |
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.
out of scope.
010_aiida_gui/readme.md
Outdated
| * `CalcJob` viewers similar to Materials Cloud. | ||
| * Provenance graph visualization or table: inputs, outputs, and dependencies. | ||
|
|
||
| * **Plugin & App Ecosystem** |
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.
out of scope.
| * **Rich Data Viewers** | ||
|
|
||
| * Raw JSON inspector for any node | ||
| * Specialized renderers: crystal structure (using weas), band structure plots, array and dict, and more. |
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.
In my note of you presentation, @giovannipizzi mention maybe not considered the actually viewer but just dump the data to the browser. Then it has full flexibility how it can be improved after and I am more confident to see the raw data structure and how the data is being validate than a "fancy" web component.
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.
That's what is proposed here:
- Raw JSON inspector for any node
and Specialized renderers, as an additional (rich) viewer for selected AiiDA data node, e.g., the AiiDA data node in theaiida-core. However, we also leave the flexibility to mount the rich viewer from the plugins.
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.
Don't "leave the flexibility", the flexibility comes when it required, for the aiida-gui, keep it small from beginning so it is finish-able. All other feature creep takes brain-energy to discuss.
| * **Core Views** | ||
|
|
||
| * **Process Table:** live view of running and completed processes, with filters, search, and status badges, and control buttons (pause, resume, kill, delete). | ||
| * **Data nodes Table:** browse Data nodes with faceted filters and custom projections. | ||
| * **Node Group Table:** create, edit, and query Groups of nodes or processes. | ||
| * **Daemon Control:** start/stop/restart the AiiDA daemon and monitor logs in real time. | ||
| * **Computer & code:** Configure and manage computers and codes. |
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.
What are correspond API endpoints? It is one or some operations requires a combined operations?
This influence the decision about whether the restAPI is better, or using GraphQL to make complex query, or using protobuff (gRPC) to have one-to-one operations bind between frontend and backend.
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.
The list of endpoints is replied in another comment.
In the current design, the FastAPI endpoints are enough to cover all the use cases. But, as written in the AEP, we need to Refactor aiida-restapi to allow query-time projections and filters.
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.
But, as written in the AEP, we need to Refactor aiida-restapi to allow query-time projections and filters.
You didn't list any detail how you gonna to refactoring, what endpoint need to change etc. Please list them out.
010_aiida_gui/readme.md
Outdated
| * **Expose** new routes for plugin registration, and app discovery. | ||
| * **Mount** plugin routers automatically by scanning entry points under `aiida_gui.plugins` and `aiida_gui.apps`. |
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.
out of scope.
| ### Backend: FastAPI & aiida-restapi | ||
|
|
||
| * **Leverage** the existing [aiida-restapi](https://github.com/aiidateam/aiida-restapi) endpoints wherever possible. | ||
| * **Refactor** aiida-restapi to allow query-time projections and filters (e.g. selecting only certain columns). |
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.
Can you list all you can expected?
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.
All the endpoints listed in another reply need to be refactored to support 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.
please list them out.
| ### Security & Deployment | ||
|
|
||
| * **Token Auth:** generate a random token at first run; all GUI requests must include `?token=` or `Authorization: Bearer`. | ||
| * **JupyterHub Integration:** provide optional flags to respect JupyterHub’s proxy authentication and mount under user’s namespace, e.g., under `/user/{name}/aiida-gui/`. |
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.
non-functional feature I believe, would be nice to have but could you give reasons why it is important to support authentication?
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 is a nice feature that is needed for the deployment. For example, people can use it inside an AiiDAlab deployment on Microsoft Azure, e.g., in the aiida tutorial.
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.
If it is something that you insist to add here, add a paragraph to argue that, please.
|
|
||
| ### Performance and Scalability | ||
|
|
||
| * **Backend:** FastAPI + Uvicorn supports async concurrency; use caching where appropriate for heavy queries. |
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 assume "caching" means the HTML caching? Be careful that can cause the inconsistent behavior when you have update the backend data at the same time.
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.
Caching where appropriate for heavy queries refers to caching on the backend side, specifically for expensive database queries. It's not related to HTML caching.
010_aiida_gui/readme.md
Outdated
| ### Plugin Architecture | ||
|
|
||
| * **Backend:** Plugins declare entry points under `aiida_gui.plugins` which provide a FastAPI Router. On startup, the core app includes these routers under `/plugins/{name}`. | ||
| * **Frontend:** Plugins publish an ESM bundle, with its entry describing routes and components to mount in the sidebar or as standalone pages. The core React app loads and mounts these dynamically. | ||
|
|
||
| ### Apps Integration | ||
|
|
||
| * **Self-contained apps** are packaged as static HTML/JS/CSS under `/apps/{app_name}/`. | ||
| * The core GUI provides a directory listing with icons and metadata for each registered app. | ||
| * Apps can still call core REST endpoints (with CORS enabled internally) and reuse shared components via the npm package. |
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.
out of scope.
|
Hi @unkcpz , thanks for reviewing this proposal.
I change the app integration to Optional. I didn't remove it entirely because it provides a broader picture of what could be possible with this extension.
I think I’ve addressed all the questions regarding the details. Just one general comment: since this is a proposal, it’s not possible to specify every detail at this stage, e.g., the exact endpoints or project/filter. These will become clearer during implementation. |
please remove it, "optional" means it is not important.
No, it is super standard thing, so everything is predictable, please try to polish the AEP as much as possible before implementation. BTW, I'll not add more comment before @giovannipizzi give it a look, I don't think me and @superstar54 can reach any concrete agreement, so the third person must make make the decision. I'd pin @giovannipizzi or @danielhollas (since you ask @superstar54 to open this during the AiiDAlab meeting if I remember correctly). |
|
I really want to hear more comment and have enough discussion on this before any tangible implementation happens. The implementation of this proposal is relatively small but the impact and the design of this is huge. I'd also recommend to give a close read at https://cloud.google.com/blog/products/api-management/understanding-grpc-openapi-and-rest-and-when-to-use-them. The key point about modern web API design is it needs to be as static as possible. If APIs is not rich enough, then it requires another work and dedicated discussion/proposal to settle it, instead of blend goal in one proposal. I'd appreciate if @giovannipizzi you can find time to have a read and add valuable comments to not let the implementation diverge too much from what it should be. also @edan-bainglass |
draftREADME.md