Update UI to indicate when a plugin attached to an entrypoint is out-of-date and provide a mechanism to sync to the latest version of plugins. #678
Conversation
|
made some modifications to use the Plugin -> PluginFiles relationship. This simplifies service layer code and I was able to remove some repeated queries. |
keithmanville
changed the title
keithmanville
changed the title
keithmanville
force-pushed
the
plugin-files-as-dependent-children-of-plugins
branch
from
d408837 to
cb14c2a
Compare
| plugin = { | ||
| entry_point_plugin.plugin.resource_id: entry_point_plugin.plugin | ||
| for entry_point_plugin in entrypoint.entry_point_plugins | ||
| }.get(plugin_id, None) |
There was a problem hiding this comment.
alternative implementation that doesn't require creating a dictionary in memory with all plugins:
[entry_point_plugin.plugin for entry_point_plugin in entrypoint.entry_point_plugins if plugin_id == entry_point_plugin.plugin.resource_id]
| utils.PluginWithFilesDict( | ||
| plugin=entry_point_plugin.plugin, | ||
| plugin_files=[ | ||
| plugin_file for plugin_file in entry_point_plugin.plugin.plugin_files |
There was a problem hiding this comment.
shouldn't need the list comprehension. Can just use plugins_files=entry_point_plugin.plugin.plugin_files.
| name=plugin.name, | ||
| description=plugin.description, | ||
| resource=plugin.resource, | ||
| creator=current_user, |
There was a problem hiding this comment.
make this a parameter to this helper function.
| @@ -776,6 +756,8 @@ def create( | |||
| new_plugin_file.parents.append(plugin.resource) | |||
| db.session.add(new_plugin_file) | |||
|
|
|||
| _update_plugin_and_file_snapshots(plugin, new_plugin_file) | |||
There was a problem hiding this comment.
Need to review if there's a better way to capture this logic, as written I'm getting confused as to what's happening.
| @@ -50,7 +50,7 @@ | |||
|
|
|||
| def export_task_engine_yaml( | |||
| entrypoint: models.EntryPoint, | |||
| entry_point_plugin_files: list[models.EntryPointPluginFile], | |||
| entry_point_plugin_files: list[models.PluginPluginFile], | |||
There was a problem hiding this comment.
Should probably rename these now that its just the plugin <-> plugin_file linking
| models.EntryPointPluginFile.entry_point_resource_snapshot_id | ||
| == entry_point_resource_snapshot_id_stmt.scalar_subquery(), | ||
|
|
||
| # Get plugins linked to the entry point |
There was a problem hiding this comment.
It might be possible to revise this query in light of the other changes.
tests/unit/restapi/v1/test_job.py
Outdated
| "queue_id": queue_snapshot_id, | ||
| "experiment_id": experiment_snapshot_id, | ||
| "entrypoint_id": entrypoint_snapshot_id, |
There was a problem hiding this comment.
This seems incorrect to use a snapshot id instead of a resource id, need to investigate.
This update takes the entry_point_plugin_files table and splits it down into 2 separate linking tables, plugin_plugin_files and entry_point_plugins. This change allows us to treat the plugin files as entirely coupled to the plugins namespace as a container, instead of their snapshotted history being tied to updates in the entrypoint.
keithmanville
force-pushed
the
plugin-files-as-dependent-children-of-plugins
branch
from
cb14c2a to
99396db
Compare
We want the UI to indicate when a plugin attached to an entrypoint is out-of-date and provide a mechanism to sync to the latest version of plugins.
This update takes the entry_point_plugin_files table and splits it down into 2 separate linking tables, plugin_plugin_files and entry_point_plugins. This change allows us to treat the plugin files as entirely coupled to the plugins namespace as a container, instead of their snapshotted history being tied to updates in the entrypoint.
In the entrypoint table and edit form, plugins should be labeled as outdated if it isn't the latestSnapshot. Beside each outdated plugin should be a "sync to latest plugin" button that should update the plugin to the latest snapshot. This is available in the entrypoint edit form and the assign plugins dialog in the entrypoints table.