Skip to content

feat: add base-path flag for sse server #137

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

Merged
merged 1 commit into from
May 21, 2025

Conversation

marin-h
Copy link
Contributor

@marin-h marin-h commented May 21, 2025

Description: Optional flag for setting a base path prefix for the SSE server.

Explanation: I needed to add this to run the MCP SSE server along with other MCP servers as containers behind a single HTTP proxy.

If there's a better way to do this let me know, I have little experience with MCP :)

fixes issue #138

@marin-h marin-h requested a review from a team as a code owner May 21, 2025 16:54
@CLAassistant
Copy link

CLAassistant commented May 21, 2025

CLA assistant check
All committers have signed the CLA.

@marin-h marin-h changed the title feat: add base-path flag for sse server feat: add base-path flag for sse server (fixes #138) May 21, 2025
@marin-h marin-h changed the title feat: add base-path flag for sse server (fixes #138) feat: add base-path flag for sse server May 21, 2025
@marin-h marin-h force-pushed the feat/sse-server-base-path branch from 726a5b1 to cc4df80 Compare May 21, 2025 17:03
@sd2k sd2k added the enhancement New feature or request label May 21, 2025
@sd2k sd2k self-assigned this May 21, 2025
@sd2k
Copy link
Collaborator

sd2k commented May 21, 2025

Thanks for the PR! Our e2e tests are failing but I think that's for an unrelated reason. Otherwise this LGTM, so I'll try to get those fixed ASAP and merge.

@sd2k
Copy link
Collaborator

sd2k commented May 21, 2025

@marin-h if you rebase on top of main then E2E tests should pass and we can get this in 🙂 thanks!

@marin-h marin-h force-pushed the feat/sse-server-base-path branch from cc4df80 to 824388e Compare May 21, 2025 17:52
@marin-h
Copy link
Contributor Author

marin-h commented May 21, 2025

@sd2k thanks for the heads up and the approval! Apparently some E2E tests are still failing. Let me know if I need to do anything else please

@sd2k
Copy link
Collaborator

sd2k commented May 21, 2025

Yeah really sorry, this is the first external PR we've had since adding more tests, I think we'll need to resolve #140 first then it'll need one more rebase 😬 I'll be able to do that first thing tomorrow morning.

@sd2k
Copy link
Collaborator

sd2k commented May 21, 2025

#141 was merged so this should be good to go after a rebase 👍 thanks for your patience!

@sd2k sd2k enabled auto-merge (squash) May 21, 2025 21:32
auto-merge was automatically disabled May 21, 2025 21:55

Head branch was pushed to by a user without write access

@marin-h marin-h force-pushed the feat/sse-server-base-path branch from 824388e to ecbf9ae Compare May 21, 2025 21:55
@sd2k sd2k merged commit 5fe4a9a into grafana:main May 21, 2025
8 of 10 checks passed
@sd2k
Copy link
Collaborator

sd2k commented May 21, 2025

Nice, I've pushed the v0.4.1 tag so this should be good to go.

@marin-h
Copy link
Contributor Author

marin-h commented May 21, 2025

Thank you!

@marin-h
Copy link
Contributor Author

marin-h commented May 23, 2025

@sd2k This is probably a noob question, but do you know when this new release 0.4.1 will be available in the Docker registry? I couldn't find any information on the Docker release process. Thanks!

@sd2k
Copy link
Collaborator

sd2k commented May 23, 2025

@marin-h oddly enough I don't know much about the Docker release process either, I think it's done automatically by Docker. It looks like they built a new latest tag 4 hours ago so I would guess this change is in there...

Edit: I just checked and it is indeed in there!

@marin-h
Copy link
Contributor Author

marin-h commented May 23, 2025

Yeah I was guessing the same thing but it is not :/

» docker run -p 8000:8000 \
  -e GRAFANA_URL=__REDACTED__ \
  -e GRAFANA_API_KEY=__REDACTED__ \
  mcp/grafana:latest -base-path /grafana
flag provided but not defined: -base-path

@marin-h
Copy link
Contributor Author

marin-h commented May 23, 2025

Oh now it is! Hahah just about time XD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants