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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃殌 SaveFile to default External Storage #1557

Merged
merged 6 commits into from Feb 3, 2022

Conversation

Aliqyan
Copy link
Contributor

@Aliqyan Aliqyan commented Oct 1, 2021

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

@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
@ReneWerner87
Copy link
Member

ReneWerner87 commented Oct 4, 2021

@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

ReneWerner87 commented Oct 11, 2021

@Aliqyan can you change this ?

@ReneWerner87
Copy link
Member

ReneWerner87 commented Oct 18, 2021

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
Member

hi019 commented Oct 20, 2021

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
14 checks passed
@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

  • please add a PR for the documentation repository

@ReneWerner87
Copy link
Member

ReneWerner87 commented Feb 3, 2022

gofiber/docs#235

NorbertHauriel pushed a commit to NorbertHauriel/fiber that referenced this issue Feb 3, 2022
* 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants