-
Notifications
You must be signed in to change notification settings - Fork 3
Graphql design doc #75
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
Conversation
|
/assign |
kerthcet
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 @wanyingz-dis
General LGTM, we can discuss more on the implementaton.
| ## 4. Schema Proposal (v0.1) | ||
| ### 4.1 Types | ||
| ``` | ||
| type Experiment { |
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.
Let's add Projects as well. Projects is for multi-tenant, for example, for multi-teams, each one will have a different project_id.
docs/graphql_design_doc.md
Outdated
| trial_id: ID! | ||
| meta: JSON | ||
| created_at: DateTime | ||
| metrics: [Metric] |
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.
Let's remove metrics from here. Metrics maybe query with trial_id, because all the metrics reflects the trial condition. We can add it in the future when needed.
docs/graphql_design_doc.md
Outdated
| experiment(uuid: ID!): Experiment | ||
| trials(experiment_uuid: ID!): [Trial] | ||
| runs(trial_uuid: ID!): [Run] | ||
| metrics(run_uuid: ID!): [Metric] |
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.
Rename to trial_metrics(trial_id). We can add run_metrics in the future. Also let's use id rather than uuid as the parameter, e.g. run_uuid -> run_id.
docs/graphql_design_doc.md
Outdated
| ### 4.2 Queries | ||
| ``` | ||
| type Query { | ||
| experiments: [Experiment] |
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.
Also projects here.
docs/graphql_design_doc.md
Outdated
| - experiments | ||
| - experiment(uuid) | ||
| - nested queries (experiment --> trials --> runs) | ||
|
|
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.
Two blank lines are enough.
docs/graphql_design_doc.md
Outdated
| Is read-only sufficient for v0.1? | ||
| (Default assumption: yes, until dashboard requires creation workflows.) | ||
| Do we want nested queries (Experiment --> Trials --> Runs) or only flat 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.
ditto
docs/graphql_design_doc.md
Outdated
|
|
||
|
|
||
| ## 10. Open Questions | ||
| Is read-only sufficient for v0.1? |
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.
yes.
docs/graphql_design_doc.md
Outdated
| ## 10. Open Questions | ||
| Is read-only sufficient for v0.1? | ||
| (Default assumption: yes, until dashboard requires creation workflows.) | ||
| Do we want nested queries (Experiment --> Trials --> Runs) or only flat 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.
Let's make it optional, the frontend can query when they want.
kerthcet
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.
let's use id rather than uuid for all the parameters.
docs/graphql_design_doc.md
Outdated
| type Query { | ||
| experiments: [Experiment] | ||
| experiment(uuid: ID!): Experiment | ||
| trials(experiment_uuid: ID!): [Trial] |
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.
We should add trial and run queries as well.
… query, replaced uuid param with id; renamed metric, updated schema, formatting
|
Thanks for the review and feedback @kerthcet , I have updated the design doc according to the review. Please take another look. |
|
/retest |
|
/lgtm |
|
Let's start to build the api server then! |
|
part of #59 |
What this PR does / why we need it
This PR introduces the design document for the first version of the graphql server.
The goal is to define how the dashboard will fetch experiment data through a simple, read-only graphql API layer. The document covers the proposed schema, API scope, directory structure, and the integration plan with fastapi and strawberry.
Why this is needed
The dashboard (Issue #61) requires a way to fetch experiments, trials, runs, and metrics efficiently.
A GraphQL endpoint allows the frontend to request exactly the data it needs with a single query per view, which avoids multiple REST calls