Skip to content

Initial upload/instantiation of custom nodes#66

Closed
reelmatt wants to merge 5 commits intomasterfrom
feature/custom-nodes
Closed

Initial upload/instantiation of custom nodes#66
reelmatt wants to merge 5 commits intomasterfrom
feature/custom-nodes

Conversation

@reelmatt
Copy link
Member

@reelmatt reelmatt commented Apr 19, 2020

We probably want to hold off on merging until we figure some issues out, but this PR has an implementation of adding custom nodes. Other approaches/questions are discussed in #67.

What this does

  • Provides a POST /workflow/custom_node endpoint that accepts a file and adds it to a custom_nodes directory within pyworkflow.
  • Provides a GET /custom_nodes endpoint that parses any files located within custom_nodes. This returns a JSON-representation of the nodes similar to the GET /nodes endpoint for "system nodes". Adds to the existing GET /nodes endpoint for system nodes, but now adds searching/parsing of files located within custom_nodes.
  • For custom nodes containing packages not currently installed/available, it adds a message telling the user to install the packages and restart the server. it compiles a list of packages and is keyed as missing_packages in the JSON response.
  • Adds creating custom nodes to the node_factory.

What this does not do/things to fix

  • Custom nodes are currently stored in pyworkflow/custom_nodes so that they can import the Node class to extend. We may or may not want to stored custom nodes separately from the package, in which case we'll need a way to reference Node.
    • The custom node path is hardcoded. We should provide a way to dynamically specify a path (within the root_dir, or elsewhere).
  • There's a lot of duplicate code to handle the uploading of the custom node file. It mirrors a lot of the upload_file methods in workflow.py and the Workflow view. If we want to pursue this approach, we should refactor uploading to accept different kinds of files and for different purposes.
  • This does not install any missing packages. Execution works for packages specified in our Pipfile (like pandas), but this should be addressed.

@reelmatt reelmatt linked an issue Apr 19, 2020 that may be closed by this pull request
Factors out some duplicate code.

PR #57 adds a `to_json()` method that might be able to replace the `extract_node_info()` method here. TBD.
`check_missing_packages()` is duplicated from `vp/views.py`. It should find a home.
@reelmatt
Copy link
Member Author

Latest commits don't change much from the initial PR. It does refactor a lot of node-parsing code (with check_missing_packages being a notable duplicated method still).

The latest commits add Custom Nodes to the same GET /nodes endpoint for the node listing. It does this through a second, separate, for loop for files within pyworkflow/custom_nodes/.

@reelmatt
Copy link
Member Author

I'm working on a refactored approach that handles Nodes differently than this PR. Closing for now, but we can revert to this state of implementation if the refactor is too problematic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants