-
Notifications
You must be signed in to change notification settings - Fork 29
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
Oncall Tools #50
Oncall Tools #50
Conversation
fd1c1ce
to
f49efbc
Compare
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.
Nice, this looks like it will be super useful! I added a few comments inline.
Makefile
Outdated
@@ -24,6 +24,10 @@ test: ## Run the Go unit tests. | |||
test-all: ## Run the Go unit and integration tests. | |||
go test -v -tags integration ./... | |||
|
|||
.PHONY: test-cloud | |||
test-cloud: ## Run only the cloud tests. | |||
go test -v -tags cloud ./tools -run TestCloudOnCall |
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.
go test -v -tags cloud ./tools -run TestCloudOnCall | |
go test -v -tags cloud ./... |
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 need the path though ./tools
as we have the mcpgrafana_test
which checks the env vars.
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've separated completely unit tests, docker based integration ones and cloud so we now run three separate commands in git actions.
docker and cloud ones require env vars which are different in each case so having them both under the same command just complicated things for no reason.
three test categories with tags maybe make a bit more sense.
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 works 🙄
@sd2k Thanks for reviewing this :) |
9092883
to
9a7cfbd
Compare
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.
LGTM! one very minor suggestion. I'll update the Ruleset to refer to the new CI jobs too.
Ah the README will need updating to refer to the new |
Co-authored-by: Ben Sully <[email protected]>
The PR adds support for Grafana OnCall features to the MCP server, including:
ListOnCallSchedules
: Lists schedules from Grafana OnCall, with team ID filteringGetOnCallShift
: Gets detailed information for a specific OnCall shift by IDGetCurrentOnCallUsers
: Shows who's currently on-call for a specific scheduleListOnCallTeams
: Lists all teams from Grafana OnCall with their detailsListOnCallUsers
: Lists all users from Grafana OnCall with their details or Get a specific user by the UserID or UsernameAlso this PR adds a "cloud test" against a specific testing instance (for now
mcptests.grafana-dev.net
)Adding support for:
depend on adding support for them to the client.
For alert groups there is already an open PR which we are pushing forward at the moment.
Before merging we need to alter the github actions required to unblock merging
At the moment we require
Test
while we are gonna need three rules specific for unit, integration and cloud.