-
Notifications
You must be signed in to change notification settings - Fork 45
Change default port from 5000 to 8000 #417
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
Conversation
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 8 changed, 0 removed
Build ID: 46666f61a5eebfd195821a4f URL: https://www.apollographql.com/docs/deploy-preview/46666f61a5eebfd195821a4f |
@@ -0,0 +1,20 @@ | |||
### Change default port from 5000 to 8000 - @DaleSeo PR #417 |
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.
nit, from the README the filename requires a prefix.
- Files must start with a prefix that indicates the classification of the changeset.
https://github.com/apollographql/apollo-mcp-server/blob/main/.changesets/README.md
Also do we consider this a breaking change?
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.
Good catch, @swcollard! Yeah, this should be a breaking change. Will update the file name.
|
||
- **Error**: "Port 5000 is already in use" | ||
- Solution: Kill any existing processes using port 5000 or specify a different port with the `transport.port` option or `APOLLO_MCP_TRANSPORT__PORT` env variable | ||
- **Error**: "Port 8000 is already in use" |
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.
Could we just remove this section? I assume this troubleshooting step was only added because of the port use region that incentivized us to change the default.
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.
This is probably still relevant. I often forgot to kill the server before starting a new one. 😂
graphql/TheSpaceDevs/README.md
Outdated
docker run \ | ||
-it --rm \ | ||
--name apollo-mcp-server \ | ||
-p 5000:5000 \ | ||
-p 8000:8000 \ | ||
-v $PWD/graphql/TheSpaceDevs:/data \ | ||
ghcr.io/apollographql/apollo-mcp-server:latest \ | ||
--http-port 5000 \ | ||
--http-port 8000 \ | ||
--schema api.graphql \ | ||
--operations operations \ | ||
--endpoint https://thespacedevs-production.up.railway.app/ |
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.
This docker run command is using CLI arguments instead of the config file which no longer work in the MCP server
graphql/TheSpaceDevs/README.md
Outdated
``` | ||
|
||
2. Add the MCP SSE port to your MCP Server configuration for the client appliction you are running. If you are running locally, the server link will be `http://127.0.0.1:5000/sse`. | ||
2. Add the MCP SSE port to your MCP Server configuration for the client appliction you are running. If you are running locally, the server link will be `http://127.0.0.1:8000/sse`. |
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.
SSE is also deprecated :(
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.
OMG, this README.md is really outdated! 🙈
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.
Two more small comments inline. Otherwise looks god
graphql/TheSpaceDevs/README.md
Outdated
-it --rm \ | ||
--name apollo-mcp-server \ | ||
-p 8000:8000 \ | ||
-v $PWD/graphql/TheSpaceDevs/config.yaml \ |
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.
-v $PWD/graphql/TheSpaceDevs/config.yaml \ | |
-v $PWD/graphql/TheSpaceDevs/config.yaml:/config.yaml \ |
I forget, does this need the path on the other side to tell docker where to mount the file?
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.
You're right and I copied that from our deploy docs! 😂 Will fix it in both places.
apollo-mcp-server/docs/source/deploy.mdx
Line 75 in 93e607b
-v <path/to>/mcp_config.yaml \ |
{ | ||
"mcpServers": { | ||
"thespacedevs": { | ||
"command": "/Users/michaelwatson/Documents/GitHub/apollographql/apollo-mcp-server/target/debug/apollo-mcp-server", |
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 also didn't realize this path was hardcoded to /Users/michaelwatson
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.
It really shows how long ago this README.md was written. We should probably delete it and just use the official docs instead.
The default server port has been changed from
5000
to8000
to avoid conflicts with common development tools and services that typically use port 5000 (such as macOS AirPlay, Flask development servers, and other local services).Migration: If you were relying on the default port 5000, you can continue using it by explicitly setting the port in your configuration file or command line arguments.