-
Notifications
You must be signed in to change notification settings - Fork 12.8k
[POC]: registrar example #108012
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: main
Are you sure you want to change the base?
[POC]: registrar example #108012
Conversation
export const registrarOnCallBaseApi = createApi({ | ||
reducerPath: 'onCallBaseApi', | ||
baseQuery: fetchBaseQuery({ | ||
baseUrl: getProxyApiUrl(), | ||
}), | ||
endpoints: () => ({}), | ||
}); | ||
|
||
export const onCallApiFromRegistrar = registrarOnCallBaseApi.injectEndpoints({ | ||
endpoints: (build) => ({ | ||
...grafanaOncallPrivateApiEndpoints(build), | ||
}), | ||
}); | ||
|
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 this part could also be automated, maybe a combination with our generator script to scaffold these 🤔
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.
Same reason as above. As far as I checked the baseUrl
cannot be manipulated after Api creation and you will want to trigger those hooks against different API base URL. So I delegated that to consumer.
Let me know if you think I missed something here though, happy to discuss as I haven't worked with Redux for couple of years now
export const registrarOnCallBaseApi = createApi({ | ||
reducerPath: 'onCallBaseApi', | ||
baseQuery: fetchBaseQuery({ | ||
baseUrl: getProxyApiUrl(), | ||
}), | ||
endpoints: () => ({}), | ||
}); |
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 wonder if this could be a part of the registrar as well?
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 challenge with baseUrl
. If we create Api in Registrar, then consumer would not be able to manipulate it after creation as far as I checked. And what really matters are typed endpoints so I decided to make it more flexible by delegating Api creation to the consumer. This way consumer can create RTK Query Api with whatever options are needed and just inject typed endpoints from Registrar.
@@ -6,6 +6,8 @@ import { CONTACT_POINTS_STATE_INTERVAL_MS } from '../utils/constants'; | |||
|
|||
import { alertingApi } from './alertingApi'; | |||
import { fetchContactPointsState } from './grafana'; | |||
import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query/react'; | |||
import { notificationsAlertingV0alpha1Endpoints } from '@grafana/hackathon-13-registrar-private/rtk-query'; |
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'm getting lots of type errors if I try to generate hooks, e.g. TS2339: Property useUpdateTimeIntervalMutation does not exist on type
, I generated the endpoints via npm run generate
in generated/typescript/private
, do I need to do anything else or are the hooks not supported?
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.
RTK client is already generated in Registrar, you don't need to do it yourself. And hooks are there when you create Api and inject endpoints. See below:
Screen.Recording.2025-07-14.at.12.47.31.mov
Happy to huddle if you want so we go through it together
@brojd sorry for delay, I talked with the team and this seems to be quite a large undertaking to take on. I'll spend more time next quarter to look more into this as we do need to figure out an approach for exposing the Grafana endpoints to plugins. I have a few concerns with this approach:
|
@Clarity-89 thanks for feedback!
This would indeed solve it but only for Grafana <-> Grafana and Grafana -> plugin communication. The huge missing part is plugin <-> plugin communication (and plugin -> Grafana but that is the smaller part).
Yes, standardized query and state management solution for Grafana and all its plugins would be an ideal world. However, the reality is that at least OnCall and Incident (and now IRM which includes both) have chosen to use http client (originally Axios, now Fetch API) + MobX (OnCall) and Tanstack Query + xstate (Incident) something like 4-5 years ago. The whole codebase is built around it and it is a large codebase. Refactoring to RTK means tons of effort. That is why today generating additional client out of the same OpenAPI schema and thanks to it having a single source of truth is already a big win and is realistic. I don't think refactoring IRM to RTK is achievable in nearest future. I should probably jump into one of the frontend meetings and we can continue the topic :) |
I think that'd be great! I think this effort might be a bit too large for one team to efficiently own. |
1000% agree, Registrar hackathon was mainly about presenting the idea. But the main point is to think long-term about a single tool that allows everyone to consume any Grafana's or its plugins' APIs in a reliable and type-safe way (I'm not talking only about frontend-to-backend which we mainly discuss in here, but also backend-to-backend). With such a solution the client generation and publishing is delegated to external tool to avoid solving this problem by each team separately (and differently) because without it, as of today, it means duplicated effort, lack of consistency and being error-prone. Such a tool should be maintained by all of us and also used by all of us. Ideally ;) |
What is this feature?
[Add a brief description of what the feature or update does.]
Why do we need this feature?
[Add a description of the problem the feature is trying to solve.]
Who is this feature for?
[Add information on what kind of user the feature is for.]
Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Please check that: