-
Notifications
You must be signed in to change notification settings - Fork 53
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
Initial commit for plugin internals refactor #763
base: main
Are you sure you want to change the base?
Conversation
bfa1029
to
f4c1af4
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #763 +/- ##
==========================================
- Coverage 77.72% 77.70% -0.02%
==========================================
Files 326 326
Lines 28575 28544 -31
==========================================
- Hits 22210 22181 -29
+ Misses 6365 6363 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
68299b4
to
84e83ea
Compare
2192994
to
85f4ebc
Compare
43b0585
to
e830c4e
Compare
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.
I noticed plugins/general/example.py
and plugins/os/unix/packagemanager.py
still have a __findable__
property, which is now no longer needed.
Another thing I noticed is that the -l
no longer lists the plugins from _os.py
.
And a third thing I noticed that if I run a namespace plugin, e.g. webserver.access, which is listed when doing target-query MyTarget -l
, before it printed an error like:
Error while parsing sysvol/windows/system32/inetsrv/config/applicationHost.config: ...
but now it errors with:
target-query: error: argument -f/--function contains invalid plugin(s): webserver.access
def _generate_long_paths() -> dict[str, FunctionDescriptor]: | ||
"""Generate a dictionary of all long paths to their function descriptors.""" | ||
paths = {} | ||
for value in _get_plugins().get("__functions__", {}).get(None, {}).values(): |
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.
Shouldn't the functions for __os__
not also be added? Otherwise they will not be found by a non-exact match in find_plugin_functions()
.
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.
I'm not sure on what the behaviour for that should be, as there are no existing unit tests for it. I assume it's not intended behaviour.
e830c4e
to
19bf4e9
Compare
Fixed package manager but we still don't want the example plugin to findable.
Should be fixed again.
I had to largely rewrite the |
19bf4e9
to
77f2144
Compare
Initial commit for the plugin internals refactor. This removes a lot of slow and hard to understand code with (hopefully) faster and easier to understand code. Also fixes some consistency issues I came across while working on this.
There are some things I want to add, but I may do those in a separate PR:
_os.py
, but then_plugin.py
or something similar (Add support for plugin directories #788)path
field, for example.This should also cover #759.
Closes #889.