Repository navigation
feat(doctor): warn when an enabled plugin isn't installed - #738
Open
dchaudhari7177 wants to merge 2 commits into
Open
dchaudhari7177 wants to merge 2 commits into
dchaudhari7177 wants to merge 2 commits into
Conversation
plugins enable only checks a name's format, so a typo or an uninstalled package stays enabled with nothing saying so. Add a non-critical Plugins row to doctor that compares the enabled names with the entry-point names installed under every Doberman group, read through _iter_entry_points so no plugin is ever loaded, and have plugins enable print the same warning when nothing installed provides the name. Closes DobermanCore#719
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #719 (and its PostHog twin #731), including the stretch goal.
What changes
doctorgets a non-critical Plugins row, last inrun_checks. It lands in the Health section, which is the default for unmapped rows. It comparesplugin_config.enabled_plugins()with the entry-point names installed under every group inregistry.ALL_GROUPS:[ ok ] Plugins: none enabled[warn] Plugins: enabled but not installed: <name>[, <name>…][ ok ] Plugins: N enabled, all installeddoctor.installed_plugin_names(). It goes throughregistry._iter_entry_pointsonly and reads.name, never.load(), so diagnosing a plugin can't import its code. This is the same approachplugins listalready takes.doberman plugins enable <name>still enables the name and exits 0. When nothing installed provides it, it also printswarning: no installed package provides a plugin named '<name>'; it stays enabled but nothing loads until one is installedto stderr.This doesn't touch #638 (the allowlist trusting a name under every entry-point group).
Tests
In
tests/unit/test_cli_doctor.py, withDOBERMAN_PLUGINS_FILEpointed attmp_pathso the real config is never touched:no_such_pluginenabled → WARNenabled but not installed: no_such_plugin, anddoberman doctorprints[warn] Plugins: enabled but not installed: no_such_plugin;_iter_entry_pointsfaked to yield an entry point whoseload()raises: the installed name counts as installed and only the missing one is reported, so the check never loads anything;plugins enable no_such_pluginwarns, and enabling an installed name prints no warning.On
mainthese give 4 failed. On this branchtest_cli_doctor.py+test_cli_plugins.pygive 58 passed, 2 skipped. ruff check and format are clean, andlint-importskeeps 5 of 5 contracts.AI assistance: I used an AI assistant while writing this change and the tests. I reviewed the diff and ran the tests above myself.