Skip to content

Allow user to specify temporary upload directory#9

Open
joaogodinho wants to merge 2 commits intodekyfin:masterfrom
joaogodinho:master
Open

Allow user to specify temporary upload directory#9
joaogodinho wants to merge 2 commits intodekyfin:masterfrom
joaogodinho:master

Conversation

@joaogodinho
Copy link
Copy Markdown

I added a parameter to the initialization of the module to allow a user to specify where to initially save uploaded files (multer dest param).

I made this as a solution to the fs.rename. If the destination path is mounted in a different device as the multer destination folder, a EXDEV is raised.

Initializes multer inside the module to allow a user to specify the destination directory.
Copy link
Copy Markdown
Owner

@dekyfin dekyfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution to make RFM-node better. I would like you to make a couple of minor changes for forward compatibility.

  1. Instead of passing the temp dir as the third parameter of the middleware, let it be an object like this: {tempDir: /path/to/temp}. This way, we can add other configurations in the future.

  2. The default tempDir should be changed from public/ to .tmp so it doesn't conflict with existing files

  3. I would appreciate it if you could add a section to the Readme that talks about the new options being introduced

Looking forward to your further contribution. Thanks

@joaogodinho
Copy link
Copy Markdown
Author

Hi,

Maybe use the already existing config file/path for it then? e.g. if there is a multer object in the config, pass it to multer? Or is that config strictly for the rich file manager options and shouldn't include implementation specific stuff?

@dekyfin
Copy link
Copy Markdown
Owner

dekyfin commented Mar 1, 2019

Exactly. The existing config is for RichFileManager and not implementation configuration. I would like us to use the third parameter for extra config

@iainbryson
Copy link
Copy Markdown
Contributor

Hello!

Good timing, I'm running into the same issue.

I'm wondering why not use a package like https://www.npmjs.com/package/mv instead (or in addition). In my case, it would be cleaner to not have the temp folder in the target directory (which is mounted as a shared docker volume and it meant to contain only the public files).

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