Skip to content

fix(backups): no full backup#105

Merged
Aditya Niraula (AdityaNPL) merged 3 commits intomasterfrom
fix/backups/no-full-backup
May 8, 2022
Merged

fix(backups): no full backup#105
Aditya Niraula (AdityaNPL) merged 3 commits intomasterfrom
fix/backups/no-full-backup

Conversation

@AdityaNPL
Copy link
Copy Markdown
Contributor

@AdityaNPL Aditya Niraula (AdityaNPL) commented May 7, 2022

  • test on a test DB

Comment thread src/AgDatabase.cs Outdated
Comment thread src/AgDatabase.cs Outdated
Copy link
Copy Markdown
Contributor

@Kaneraz Ryan Clare (Kaneraz) left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct.

When throwing a specific exception I prefer to make a new type, and catch it rather than just throw exception.

Also, it can be expected that a backup may not exist on each node, so the interface should change to be nullable. Save the exceptions for when something unexpected happens.

@AdityaNPL
Copy link
Copy Markdown
Contributor Author

Ryan Clare (@Kaneraz) - I agree, maybe we can return decimal? or some other nullable value instead.
I need this fix right now so will merge it in and follow up with a refactor tomorrow.

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.

4 participants