API: Add by UID routes for data sources #29884
Conversation
kylebrandt
changed the title
There was a problem hiding this comment.
Overall it looks good but I do feel that the amount of duplication is a bit too much. Would try to share the query operation (see GetDashboardQuery for reference it allows fetching dashboard by id, or uid or slug). And on the API handler it feels like we can also share more
kylebrandt
requested review from
sunker,
jesusvazquez,
oddlittlebird and
achatterjee-grafana
and removed request for
a team
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
kylebrandt
changed the title
pkg/api/datasources.go
Outdated
| ds, err := getRawDataSourceByUID(uid, c.OrgId) | ||
| if err != nil { | ||
| return Error(400, "Failed to delete datasource", nil) | ||
| } | ||
|
|
||
| if ds.ReadOnly { | ||
| return Error(403, "Cannot delete read-only data source", nil) | ||
| } | ||
|
|
||
| cmd := &models.DeleteDataSourceCommand{UID: uid, OrgID: c.OrgId} |
There was a problem hiding this comment.
Nit. This part and below could be reused by DeleteDataSourceByUID and DeleteDataSourceById if extracted to func deleteDataSource(cmd *models.DeleteDataSourceCommand) Response. But could be fixed later if you want to get this merged fast.
Nit. If the data source is not found in getRawDataSourceByUID that should probably return a 404 not found instead. Same applies to DeleteDataSourceById.
There was a problem hiding this comment.
Fixed for 404, will save other for possible future work. Seems we have mix of responses. May want to start returning the UID (which requires the getCmd results), so will save that for possible future work focusing on api consistency.
kylebrandt
changed the title
- also add Get by UID+OrgID to datasource cache - Refactor backend commands for Delete and Get to be unified
What this PR does / why we need it:
Adds the ability to get a data source by UID from SQL and via the datasource cache. This is so alerting ng will be able to store data sources by uid instead of id so it works with provisioning.
We may also switch the frontend to operate by UID, so this sets us up for that.
For requests by ID, an additional cache key to the datasource based on the UID is added. For requests by UID, the opposite is also true - an additional id cache key is added.
Also adds HTTP methods for getting + deleting via UID, and adds UID as a property to the response when listing datasources.
Which issue(s) this PR fixes:
Fixes #29852
Special notes for your reviewer: