Skip to content

Makes BackupKeyspaceSchema return an error on failure#23

Merged
maorfr merged 1 commit intonuvo:masterfrom
gyant:ryanthompson/fix-schema-backup-error
Sep 22, 2020
Merged

Makes BackupKeyspaceSchema return an error on failure#23
maorfr merged 1 commit intonuvo:masterfrom
gyant:ryanthompson/fix-schema-backup-error

Conversation

@gyant
Copy link
Copy Markdown
Contributor

@gyant gyant commented May 5, 2020

Ran into an issue where a coworker had introduced some s3 bucket policies that strictly enforced encryption headers. They weren't required because the backup bucket had default server-side encryption enabled and the aws sdk took care of all of that on the backend. However, since this return block did not include the error, the app kept on rolling into the copy step and we were getting 404 errors in the logs because the entire path prefix had been wiped out because of this return. Took me all day to narrow down so thought I'd PR this to save someone else the headache if they ever run into this issue.

Ran into an issue where a coworker had introduced some s3 bucket policies that strictly enforced encryption headers. They weren't required because the backup bucket had default server-side encryption enabled and the aws sdk took care of all of that on the backend. However, since this return block did not include the error, the app kept on rolling into the copy step and we were getting 404 errors in the logs because the entire path prefix had been wiped out because of this return. Took me all day to narrow down so thought I'd PR this to save someone else the headache if they ever run into this issue.
@maorfr
Copy link
Copy Markdown
Contributor

maorfr commented May 5, 2020

Thanks for this!

This seems harmless enough, and thanks for the explanation.

@tomklino objections to merging this? if none, I'll merge within a couple of days.

@lanefu
Copy link
Copy Markdown

lanefu commented Sep 21, 2020

@maorfr still make sense to merge?

@maorfr maorfr merged commit bdb22f5 into nuvo:master Sep 22, 2020
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.

3 participants