-
Notifications
You must be signed in to change notification settings - Fork 336
Containerising inspector #257
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
base: main
Are you sure you want to change the base?
Conversation
We really need something in the "how has this been tested" section of these PRs, particularly if it hooks into CI. |
I'll update that section, but it's mainly running a docker build and starting a container. Testing the CI pipeline is a bit harder, since it's artifact publishing, so I'm just using the instructions from the GitHub docs. |
Got it. But I don't want to break the inspector with an untested workflow. Can you please test it in your fork and take screenshots of every step? |
@cliffhall I've updated the issue description with how I've tests it and links to the Actions run on my fork that I used. Here's some screenshot of the image on my fork: And the workflow run (sorry, it's pretty long, so I'll break it a bit 🤣): |
@aaronpowell thanks for doing that testing, I really appreciate it. Will give it closer eyes soon. |
Let me know if you need any more details on the workflow or the life cycle. |
Fixes #237
Motivation and Context
I want to be able to use a container version of the inspector rather than via npx.
How Has This Been Tested?
I merged the branch into
main
on my fork and created a tag so I could publish a release. Here's the workflow run (ignore the failingpackage
job, I can't publish to npm) and here's the published imageNote: since the workflow is configured to publish to GitHub Packages not Docker Hub the
GITHUB_TOKEN
needs to be able to write packages. This is configured in the workflow, but you also need to update the repo settings to allow the workflow to request a token with write permissions:Breaking Changes
Types of changes
Checklist
Additional context
Currently, the image only has the nodejs runtime in it, so it doesn't support running stdio servers with python/.NET/Java/etc. Instead, they would probably be handled as different tags. My primary focus is on supporthing servers that are SSE (and streamable http in the future)