-
Notifications
You must be signed in to change notification settings - Fork 130
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
Update/download files through integration #365
Conversation
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.
Looks good. For now, and the type of use cases that are considered, having these files temporarily in memory should work fine. You might want to monitor this in production and if necessary apply changes to accommodate increased memory usage (for instance tweaking gunicorn config).
try: | ||
files_to_send[field_name] = (file_name, self.params["files"][file_name]) | ||
except KeyError: | ||
return ( |
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.
Good to see that incorrect file names are caught here with proper feedback for the user. Additionally, you could choose to validate this when saving the integration.
This allows users to set up manifests with download and upload options. You will first have to download a file before you can upload it to a different endpoint.
Especially helpful for moving files between storage providers.