-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
✨ Allow for specifying the S3 Storage Class #205
base: master
Are you sure you want to change the base?
Conversation
Requires that dokku/docker-s3backup#12 be merged
rm -f "$SERVICE_BACKUP_ROOT/AWS_SECRET_ACCESS_KEY" | ||
rm -f "$SERVICE_BACKUP_ROOT/AWS_DEFAULT_REGION" | ||
rm -f "$SERVICE_BACKUP_ROOT/AWS_SIGNATURE_VERSION" | ||
rm -f "$SERVICE_BACKUP_ROOT/ENDPOINT_URL" |
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.
Why this change?
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.
Because the auth command is no longer the only one using that directory, leaving it the way it was would also delete the S3_STORAGE_CLASS.
@@ -226,7 +231,7 @@ service_backup_auth() { | |||
declare SERVICE="$1" AWS_ACCESS_KEY_ID="$2" AWS_SECRET_ACCESS_KEY="$3" AWS_DEFAULT_REGION="$4" AWS_SIGNATURE_VERSION="$5" ENDPOINT_URL="$6" | |||
local SERVICE_BACKUP_ROOT="$PLUGIN_DATA_ROOT/$SERVICE/backup" | |||
|
|||
mkdir "$SERVICE_BACKUP_ROOT" | |||
mkdir -p "$SERVICE_BACKUP_ROOT" |
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.
Was this necessary?
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.
Otherwise it will fail upon trying to create the dir if it already existed
README.md
Outdated
@@ -21,6 +21,7 @@ postgres:app-links <app> # list all postgres service l | |||
postgres:backup <service> <bucket-name> [--use-iam] # creates a backup of the postgres service to an existing s3 bucket | |||
postgres:backup-auth <service> <aws-access-key-id> <aws-secret-access-key> <aws-default-region> <aws-signature-version> <endpoint-url> # sets up authentication for backups on the postgres service | |||
postgres:backup-deauth <service> # removes backup authentication for the postgres service | |||
postgres:backup-s3class <service> <s> <-storage-class> # sets the S3 storage class to be used for backups on the postgres service |
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.
Can you manually regenerate the readme via make README.md
?
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.
That's the way it generates, I can fix the output but idk if there's an issue with the parsing or my subcommand.
common-functions
Outdated
local SERVICE_BACKUP_ROOT="$PLUGIN_DATA_ROOT/$SERVICE/backup/" | ||
|
||
mkdir -p "$SERVICE_BACKUP_ROOT" | ||
echo "$S3_STORAGE_CLASS" >"$SERVICE_BACKUP_ROOT/S3_STORAGE_CLASS" |
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.
I really need to transfer all of this to the new properties system...
Requires that dokku/docker-s3backup#12 be merged