-
Notifications
You must be signed in to change notification settings - Fork 1k
Add ability to upload logical backup to gcs #1173
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
Conversation
I'm fine with these changes. Just wonder if the AWS default should be called |
👍 |
S3AccessKeyID string `json:"logical_backup_s3_access_key_id,omitempty"` | ||
S3SecretAccessKey string `json:"logical_backup_s3_secret_access_key,omitempty"` | ||
S3SSE string `json:"logical_backup_s3_sse,omitempty"` | ||
GoogleApplicationCredentials string `json:"logical_backup_google_application_credentials,omitempty"` |
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.
Should this be ..Path
or ...File
?
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.
@Jan-M Hm. Technically it should be Path
, since it refers to a path where google credentials are.
But, in AWSGCPConfiguration
it is also simply gcp_credentials
:
GCPCredentials string `json:"gcp_credentials,omitempty"` |
So if we were to update it here, we should probably update it there too? But that will be a breaking change then.
What do you think?
Even if this is not directly related, what about using a tool like rclone to export the backups ? This would make it possible to support S3 / GCS / Openstack Swift / ... transparently by just injecting the required env vars ? |
👍 |
@cambierr sure in theory we / I do consider it desireable that we have job/pod specs that others can leverage by providing customer docker images. |
This is based on:
#873
Resolved conflicts that original PR had.
@FxKu Would appreciate if you will have time to take a look at this one and hopefully approve it for merging.