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

[PostgresAdmin] Fix backup_type being ignored #405

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Nov 7, 2018

This is a FIXUP to #402

With the previous implementation, the code would always attempt to find the magic number of the file for a pg_dump before checking if the backup_type value was instead a :basebackup. When streaming a file, this would cause it to fail farther down the line since it would have strip some of the content off from the beginning of the file, invalidating it as a proper backup.

This change fixes by forcing the check for the backup type first to avoid reading the file data if possible, and then by inspecting the file contents when as a backup option.

@NickLaMuro
Copy link
Member Author

NickLaMuro commented Nov 7, 2018

Going to add a test to this real quick, since this took me WAY to long to debug yesterday... 😩

Update: Done! (that was easier than expected 😏 )

With the previous implementation, the code would always attempt to find
the magic number of the file for a `pg_dump` before checking if the
`backup_type` value was instead a `:basebackup`.  When streaming a file,
this would cause it to fail farther down the line since it would have
strip some of the content off from the beginning of the file,
invalidating it as a proper backup.

This change fixes by forcing the check for the backup type first to
avoid reading the file data if possible, and then by inspecting the file
contents when as a backup option.
@NickLaMuro NickLaMuro force-pushed the fix-backup-type-checking-for-postgres-admin branch from caf0f38 to 5b9e017 Compare November 7, 2018 21:54
@NickLaMuro
Copy link
Member Author

@miq-bot assign @carbonin
@miq-bot add_label bug, hammer/yes

@carbonin carbonin merged commit 2276ef7 into ManageIQ:master Nov 8, 2018
@carbonin carbonin self-assigned this Nov 8, 2018
@carbonin carbonin added this to the Sprint 99 Ending Nov 19, 2018 milestone Nov 8, 2018
simaishi pushed a commit that referenced this pull request Nov 12, 2018
…postgres-admin

[PostgresAdmin] Fix backup_type being ignored

(cherry picked from commit 2276ef7)
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.

3 participants