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

Support multiple s3 storage configuration #1183

Merged
merged 1 commit into from
Dec 19, 2021

Conversation

mialinx
Copy link
Contributor

@mialinx mialinx commented Dec 19, 2021

After merging wal-g/storages#45 we are unable to configure multiple s3 storages as it needed in copy commands.

Suggested fix #1050 works, but it's dirty. Call to the FolderFromConfig pollutes the environment, making impposible to any configuration function (like ConfigureCompressor) after it.

Solution:

  • Wal-g config file should be the first priorty configuration source.
  • Set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY only if they are not empty, otherwise fallback to the enviroment (and other storage dependent) configuration defaults
  • Do not overwrite the environment in FolderFromConfig

After merging wal-g/storages#45 we are unable to
configure multiple s3 storages as it needed in copy commands.

Suggested fix wal-g#1050 works, but it's
dirty. Call to the `FolderFromConfig` pollutes the environment,
making impposible to any configuration function
(like `ConfigureCompressor`) after it.

Solution:
* Wal-g config file should be the first priorty configuration source.
* Set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY only if they are not
empty, otherwise fallback to the enviroment (and other storage
dependent) configuration defaults
* Do not overwrite the environment in `FolderFromConfig`
@mialinx mialinx requested a review from a team as a code owner December 19, 2021 12:43
@mialinx
Copy link
Contributor Author

mialinx commented Dec 19, 2021

@rocket357, @andersosthus, you may be interested/affected by these changes

@serprex
Copy link
Contributor

serprex commented Dec 19, 2021

Also update docs/STORAGES.md

@mialinx mialinx merged commit 9cf553c into wal-g:master Dec 19, 2021
@andersosthus
Copy link

@paalkr just a heads up about this change

@usernamedt usernamedt mentioned this pull request Feb 4, 2022
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.

None yet

4 participants