Skip to content
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

Jump to bottom

🚀 SaveFile to default External Storage #1557

Merged
merged 6 commits into from Feb 3, 2022
Merged

🚀 SaveFile to default External Storage #1557

merged 6 commits into from Feb 3, 2022

Conversation

Aliqyan
Copy link
Contributor

@Aliqyan Aliqyan commented Oct 1, 2021 •
edited

Allow client users to create a Storage interface for a default cloud based storage (like s3), and automatically save the file to the configured external file storage system if one exists.

S3 storage Addition (in GoFiber/Storage): gofiber/storage#227

Feature was suggested here: https://discordapp.com/channels/704680098577514527/848707314180423750/869831718876643388

@welcome
Copy link

welcome bot commented Oct 1, 2021

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@pcen pcen mentioned this pull request Oct 1, 2021
Closed
@Aliqyan Aliqyan changed the title [WIP] SaveFile to default External Storage 🚀 SaveFile to default External Storage Oct 1, 2021
@ReneWerner87 ReneWerner87 linked an issue Oct 4, 2021 that may be closed by this pull request
🚀 Add Configurable Default ExternalStorage for Ctx.SaveFile #1558
Closed
@ReneWerner87
Copy link
Member

ReneWerner87 commented Oct 4, 2021 •
edited

@Aliqyan
not quite sure if we should extend this method and the config with it, rather in favor of a new method which is used for this purpose

e.g. SaveFileToStorage

@ReneWerner87
Copy link
Member

@Aliqyan can you change this ?

@ReneWerner87
Copy link
Member

thanks for the change, now we just need unittest for the new function

also don't know if it wouldn't be better to just introduce a third parameter or a parameter object instead of the property on the app, because the external storage is currently only used there

what do you think ? @Fenny @hi019 @gofiber/maintainers

@hi019
Copy link
Contributor

hi019 commented Oct 20, 2021 •
edited

I don't have a preference. Using a parameter would allow consumers to save files to multiple different storage interfaces, but that could also be done by making the ExternalStorage a map[string]Storage

@ReneWerner87 ReneWerner87 merged commit 39a503c into gofiber:master Feb 3, 2022
@welcome
Copy link

welcome bot commented Feb 3, 2022

Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@ReneWerner87
Copy link
Member

ReneWerner87 commented Feb 3, 2022 •
edited

  • please add a PR for the documentation repository

@ReneWerner87
Copy link
Member

gofiber/docs#235

NorbertHauriel pushed a commit to NorbertHauriel/fiber that referenced this pull request Feb 3, 2022
@Aliqyan @efectn @NorbertHauriel
🚀 SaveFile to default External Storage (gofiber#1557)
2dcc9db 
* fiber change

* reference correct storage in SaveFile

* seperate logic to new function

* Change storage defination style, fix tests on go1.14

* Add unit test.

Co-authored-by: Alex Bakker <abakks@hotmail.com>
Co-authored-by: M. Efe Çetin <efectn@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ReneWerner87 ReneWerner87 ReneWerner87 approved these changes

Assignees
No one assigned
Labels
✏️ Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🚀 Add Configurable Default ExternalStorage for Ctx.SaveFile
4 participants
@Aliqyan @ReneWerner87 @hi019 @efectn