Fix unscheduling backups for some config var configurations #1539
Conversation
For some applications with aliased config vars, it's possible that the unschedule command can be too stringent in verifying the database to unschedule, making it impossible to unschedule backups without manually calling the API. This makes the unschedule verification much simpler.
c38c3c7
to
05310b4
Compare
woot woot. Thanks @uhoh-itsmaciek! Yeah, much simpler and also you know, correct. So +1! |
@@ -513,12 +513,15 @@ def unschedule_backups | |||
db = shift_argument | |||
validate_arguments! | |||
|
|||
if db.nil? | |||
abort("Must specify database to unschedule backups for") | |||
end |
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.
If there is only one schedule, should we accept no db
here?
For this message, I'd add a hint:
Must specify a database. Run `heroku help pg:backups` for usage information.
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 think it does make sense to require an argument. Maybe instead of pointing people to help
, we should just list what's available to unschedule instead?
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 agree we should require an argument. :)
@hgmnz @chadbailey59 Improved the error message / error handling. How's this look? |
Just got a ticket for this—any sign of merging? |
@uhoh-itsmaciek @tef The error messages and handling look good to me. |
Fix unscheduling backups for some config var configurations
For some applications with aliased config vars, it's possible
that the unschedule command can be too stringent in verifying
the database to unschedule, making it impossible to unschedule
backups without manually calling the API.
This makes the unschedule verification much simpler.
/cc @chadbailey59 @neovintage @hgmnz