-
Notifications
You must be signed in to change notification settings - Fork 41
Fix plugin lookup, refactor library enumeration, and update pytest #566
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: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Does this work with the correct python executable if we have multiple of them locally?
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.
yes this enumerates the distributions visible to the interpreter running the server (the same as sys.executable)!
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.
All changes work for me but I think the plugin one might be unnecessary as that function doesn't get used
| return sqlalchemy_to_dict(plugin) if plugin else None | ||
|
|
||
|
|
||
| async def save_plugin(name: str, type: str): |
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 think this is a leftover function, we dont use this one anymore or the associated parent function, I think?
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.
it gets used in plugin_download but maybe that endpoint is no longer used? mainly there was a test associated with this function that was failing locally
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.
The plugin_download function is deprecated now. Do we also get rid of the test then?
| return r | ||
|
|
||
|
|
||
| @router.get("/python_libraries") |
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.
Tried this change and it worked for me
Summary
This PR introduces a bug fix for plugin database lookups, a performance and reliability refactor for Python package enumeration, and an enhancement to the pytest configuration.
Changes
pytest.ini:norecursedirsto the[pytest]configuration. This prevents the test collector from scanning virtual environments (*/venv/*,*/site-packages/*) and temporary directories (test/tmp), improving performance and preventing the execution of third-party tests.transformerlab/db/db.py:save_pluginfunction was modified to query for plugins usingselect(Plugin).where(Plugin.name == name)instead ofsession.get(Plugin, name).namefield was the primary key. This change corrects the lookup logic to query against the uniquenamecolumn, resolving an issue where existing plugins could not be found and updated.transformerlab/routers/serverinfo.py:get_python_library_versionsendpoint was updated to useimportlib.metadatato enumerate installed packages.subprocesscall topip, which improves endpoint performance and security. The previouspipmethod is retained as a fallback for increased resilience.