Skip to content

Upload data files during node config#41

Merged
reddigari merged 8 commits intomasterfrom
feature/file-upload
Apr 7, 2020
Merged

Upload data files during node config#41
reddigari merged 8 commits intomasterfrom
feature/file-upload

Conversation

@reddigari
Copy link
Collaborator

@reddigari reddigari commented Apr 5, 2020

(This includes the commits from #40, so it will look much cleaner after that one is merged.)

When file is selected from configuration menu, it is uploaded to the server and saved with the filename: <node ID>-<original name>.<original_extension>. The configuration form submit button is disabled until the server responds. The parameter to which the file corresponds (e.g. filepath_or_buffer is given the value of the filename on the server.

This brought to my attention a couple of execution bugs:

  • The filename saved in the node options doesn't have the /tmp prefix, so pandas cannot find it. We will need to add a step in the node update view that replaces the value with the fully qualified FileStorage name.
  • The description field is inserted in the node options by the browser, and since the pandas function is called with (**options), it complains that description is not a valid keyword arg.

These issues are causing me to cool on the "pass the options dict as kwargs" strategy overall. Thoughts?

@reddigari reddigari changed the title Feature/file upload Upload data files during node config Apr 5, 2020
@reddigari
Copy link
Collaborator Author

  • Altered create_node view helper to replace options representing files with the fully-specified FileStorage path so execute() can find the file
  • Fixed ReadCsvNode.execute() to not pass the description as a keyword arg.

Execution works!! (at least on ReadCsvNode)

@reddigari reddigari merged commit bebadbd into master Apr 7, 2020
@reddigari reddigari deleted the feature/file-upload branch April 8, 2020 20:37
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.

3 participants