-
-
Notifications
You must be signed in to change notification settings - Fork 57
Split global dependency error messages to distinguish missing adapters from version mismatches #3085
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: master
Are you sure you want to change the base?
Conversation
Changed "Adapter dependency not fulfilled on any host" to "Global adapter dependency not fulfilled" to make the error message clearer and less confusing. The word "any" in the original message suggested the dependency only needed to be on one host, when in fact global dependencies require all instances to satisfy the version requirement. Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
Changed from "Global adapter dependency not fulfilled" to "Adapter dependency not fulfilled on all hosts where adapter is installed" to provide clearer guidance to users about what they need to check in multihost scenarios. Based on feedback from @Apollon77, the message now specifically tells users to check all hosts where the adapter is installed, which is the actual requirement for global dependencies. Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
lgtm |
I still find this message confusing, if the dependency eg ist not installed at all |
The message will not be displayed if the adapter is not installed at all at a slave. |
Will be displayed whenever global dep ist not fulfilled, could be not installed at all but could also be wrong version on some host.. |
This message will not be displayed if i.e. admin 7.0.0 is required,admin 7.x.x is installed on master buet admin is NOT installed on slave. Message will be displayed if admin 7.x.x is installed on master and admin 6.x.x is installed on slave. This behavior is desired and ok. Onle the current message that admin dependency is not fullfilled on ANY host is wrong. In the second example dependency is fullfuöled on ANY host (master) but not on ALL host ( the slave) |
And it is displayed if no admin is installed at all and then it becomes a weird message |
OK - I see Maybe we need two diffrent messages
|
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.
lgtm
Alright then lets split the error message variants @copilot The error message is still confusing in the case that the global dependency is missing at all. Please adjust checkVersion to throw different error instance types (create them) for not found and version-mismatch cases and then ensure that the two cases have meaningful error messages |
Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
I've updated the error handling to provide specific error messages for different scenarios. Created separate error classes ( Now the error messages are:
This makes it much clearer for users to understand whether they need to install a missing adapter or update an existing one to meet version requirements. Commit: 51c56c0 |
@foxriver76 Please check again |
The error message for global adapter dependencies was confusing and not actionable for users, especially when adapters were completely missing versus having version mismatches. The controller would show the same generic message for both scenarios:
This change implements distinct error handling to provide specific, actionable messages based on the actual problem:
Enhanced Error Handling
For missing adapters:
For version mismatches:
Implementation Details
AdapterNotFoundError
andAdapterVersionMismatchError
to distinguish between different failure typesExample Scenarios
Missing Adapter
When an adapter dependency is not installed at all:
Version Mismatch in Multihost
An adapter requires
admin >= 7.4.10
as a global dependency, but the system hasadmin 7.2.6
on a slave host:Benefits
The implementation uses surgical changes with custom error classes to maintain backward compatibility while providing much clearer user guidance.
Fixes #3037.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.