Skip to content
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

Playlist: Remove unused/deprecated api and unused wrapper #75503

Merged
merged 6 commits into from
Sep 27, 2023

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Sep 27, 2023

This removes the deprecated /playlists/{uid}/dashboards that has not been used for a year+, and the unused dual write EntityAPI service.

Release notice breaking change

The deprecated /playlists/{uid}/dashboards API endpoint has been removed. Dashboard information can be retrieved from the /dashboard/... APIs.

@ryantxu ryantxu requested a review from a team as a code owner September 27, 2023 00:51
@ryantxu ryantxu requested review from mildwonkey, nikimanoledaki and suntala and removed request for a team September 27, 2023 00:51
@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.2.x milestone Sep 27, 2023
@ryantxu ryantxu added no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels Sep 27, 2023
@ryantxu ryantxu changed the title Playlist: Remove deprecated api and unused wrapper Playlist: Remove unused/deprecated api and unused wrapper Sep 27, 2023
return result
}

// Deprecated -- the frontend can do this better
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the frontend was changed to skip this ~2 years ago. This function requires the playlist CRUD and dashboard search to know about eachother.

@ryantxu ryantxu added add to changelog and removed no-changelog Skip including change in changelog/release notes labels Sep 27, 2023
Copy link
Contributor

@papagian papagian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this route was included in the docs and the OpenAPI spec, maybe we should mark it as breaking change as well.

@@ -7677,40 +7677,6 @@
}
}
},
"/playlists/{uid}/dashboards": {
Copy link
Contributor

@papagian papagian Sep 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have to remove it also from
public/openapi3.json

note that they can be re-created both by using:

# OpenAPI generation
Developers can re-create the OpenAPI v2 and v3 specifications using the following command:
```bash
make swagger-clean && make openapi3-gen
```
They can observe its output into the `public/api-merged.json` and `public/openapi3.json` files.
Finally, they can browser and try out both the OpenAPI v2 and v3 via the Swagger UI editor (served by the grafana server) by navigating to `/swagger-ui` and `/openapi3` respectivally.

@ryantxu ryantxu added the breaking change Relevant for changelog generation label Sep 27, 2023
@ryantxu ryantxu enabled auto-merge (squash) September 27, 2023 14:36
@ryantxu ryantxu merged commit bbdd1fc into main Sep 27, 2023
15 checks passed
@ryantxu ryantxu deleted the playlist-cleanup branch September 27, 2023 15:28
otilor pushed a commit to otilor/grafana that referenced this pull request Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants