Issue 2680

[4t8dd]

fix of issue #2680

[4t8dd]

@ChrisJBurns Hi Chris, please help review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.09%. Comparing base (5093c91) to head (7594850).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2757      +/-   ##
==========================================
+ Coverage   56.07%   56.09%   +0.01%     
==========================================
  Files         319      319              
  Lines       30697    30708      +11     
==========================================
+ Hits        17214    17226      +12     
+ Misses      12002    12001       -1     
  Partials     1481     1481              

☔ 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.

[jhrozek]

@amirejaz 👀

[amirejaz]

@amirejaz 👀

From the logs point of view this looks good, and it won’t notify the user as this will log in the detach proxy. However, I was hoping for a more interactive way to let the user know. Maybe we need to set a state that the UI can use to prompt the user to choose another authentication method. In CI, the user can see this state via thv list.

[amirejaz]

@4t8dd This is a good improvement. I’m not entirely sure that adding a DCRUnsupported (or equivalent) state is the best approach. We can go ahead and merge this and continue discussing a more interactive solution. In any case, we will still need to update the logs.

@dmjb any thoughts?

[4t8dd]

@4t8dd This is a good improvement. I’m not entirely sure that adding a DCRUnsupported (or equivalent) state is the best approach. We can go ahead and merge this and continue discussing a more interactive solution. In any case, we will still need to update the logs.

@dmjb any thoughts?

yes, let's have this at this time and I will keep working and improve this.

[amirejaz]

Thanks, @4t8dd! I’ve merged this PR and left the issue open. I’ll discuss the proper approach with the team and share the details here once we have a direction. If you’re interested in implementing it, you’re more than welcome to jump in.

In the meantime, here’s a collection of good first issues you might enjoy exploring: https://github.com/stacklok/toolhive/issues?q=is%3Aissue%20state%3Aopen%20label%3A%22good%20first%20issue%22

Thanks again!

[ChrisJBurns]

Good stuff, thanks @4t8dd for the contribution!! 🚀

[4t8dd]

Thanks, @4t8dd! I’ve merged this PR and left the issue open. I’ll discuss the proper approach with the team and share the details here once we have a direction. If you’re interested in implementing it, you’re more than welcome to jump in.

In the meantime, here’s a collection of good first issues you might enjoy exploring: https://github.com/stacklok/toolhive/issues?q=is%3Aissue%20state%3Aopen%20label%3A%22good%20first%20issue%22

Thanks again!

yes, definitly. I would like to figiure this out so that I can deploy this in our env for the test.
Any info, please be kind share. Thanks.