Skip to content

Conversation

goshlanguage
Copy link
Contributor

@goshlanguage goshlanguage commented Aug 8, 2025

I needed to add colima in order to use thv in my current setup. Opening as draft as I'm traveling as I write this PR and haven't given due diligence of going through the Dev Guide. Hoping to share for guidance.

Before:

bin/thv client setup                       
A new version of ToolHive is available: v0.2.5
Currently running: v8ef1a70-dirty
Error: failed to create client manager: no supported container runtime available: container runtime not found

After:

bin/thv client setup
A new version of ToolHive is available: v0.2.5
Currently running: v8ef1a70-dirty
Successfully registered client: vscode
Successfully registered client: cursor
10:02AM INF Creating new client config file at /Users/rhartje/Library/Application Support/Code/User/mcp.json
10:02AM INF Creating new client config file at /Users/rhartje/.cursor/mcp.json

@dmjb
Copy link
Member

dmjb commented Aug 13, 2025

@goshlanguage This makes sense to me. Feel free to update the branch and take it out of draft status.

@goshlanguage goshlanguage marked this pull request as ready for review August 13, 2025 17:16
@JesseObrien
Copy link

JesseObrien commented Aug 19, 2025

I added a PR that does almost exactly the same thing. I wanted to 👍 the support for this PR. I updated all of the docs to include Colima in mine.

@eleftherias eleftherias self-assigned this Aug 21, 2025
Copy link
Member

@eleftherias eleftherias left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @goshlanguage! I've added a suggestion inline about how this could be simplified.

@goshlanguage
Copy link
Contributor Author

goshlanguage commented Aug 26, 2025

@eleftherias seems that your suggestion then will only work for Colima on mac, and not colima on linux. Are you sure that's what we want? Means I still can't use this on my linux environments.

edit: I haven't confirmed, so I will test this. I do like less code

@goshlanguage
Copy link
Contributor Author

yeah unfortunately it looks like that's not a compatible approach:

➜  toolhive git:(support-colima) ./bin/thv client setup
A new version of ToolHive is available: v0.2.13
Currently running: vf0c65e0
Error: failed to create client manager: no supported container runtime available: container runtime not found

Copy link
Member

@eleftherias eleftherias left a comment

Choose a reason for hiding this comment

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

Thanks for trying it out on Linux! We do want it to work on both.
With the current proposal does it work on Linux, or is there still more to be done?

@goshlanguage
Copy link
Contributor Author

Thanks for trying it out on Linux! We do want it to work on both. With the current proposal does it work on Linux, or is there still more to be done?

It worked with the original implementation. What do we want to do here? Restore that?

@eleftherias
Copy link
Member

Thanks for trying it out on Linux! We do want it to work on both. With the current proposal does it work on Linux, or is there still more to be done?

It worked with the original implementation. What do we want to do here? Restore that?

Yes, let's get back to the one that worked on Linux as well.

Copy link

codecov bot commented Sep 5, 2025

Codecov Report

❌ Patch coverage is 0% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.45%. Comparing base (d8461be) to head (4327cf8).
⚠️ Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
pkg/container/docker/sdk/client_unix.go 0.00% 32 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1332      +/-   ##
==========================================
- Coverage   38.51%   38.45%   -0.07%     
==========================================
  Files         174      174              
  Lines       20233    20265      +32     
==========================================
  Hits         7792     7792              
- Misses      11870    11902      +32     
  Partials      571      571              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 35.866% (-0.1%) from 35.987%
when pulling 4327cf8 on goshlanguage:support-colima
into a0903c7 on stacklok:main.

Copy link
Member

@eleftherias eleftherias left a comment

Choose a reason for hiding this comment

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

Thanks for you effort on this @goshlanguage!

@eleftherias eleftherias merged commit 05edfa1 into stacklok:main Sep 8, 2025
14 of 16 checks passed
@goshlanguage goshlanguage deleted the support-colima branch September 8, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants