fixes #8549 - Fixes subscription error messages for act key, BZ 1154619 #4839
Conversation
| @@ -213,6 +213,14 @@ def copy(new_name) | |||
| new_key | |||
| end | |||
|
|
|||
| def subscribe(pool_id, quantity = 1) | |||
| Resources::Candlepin::ActivationKey.add_pools self.cp_id, pool_id, quantity | |||
There was a problem hiding this comment.
Mind wrapping this args in parens?
theforeman-bot
added
Needs testing
Needs re-review
and removed
Waiting on contributor
labels
| def subscribe(pool_id, quantity = 1) | ||
| Resources::Candlepin::ActivationKey.add_pools(self.cp_id, pool_id, quantity) | ||
| rescue RestClient::ResourceNotFound, RestClient::BadRequest => e | ||
| raise JSON.parse(e.response)['displayMessage'] |
There was a problem hiding this comment.
@daviddavis would fail be better here?
There was a problem hiding this comment.
Negatory. raise is the appropriate call since it's called inside a rescue.
There was a problem hiding this comment.
Here's Jim Weirich:
An aside, because I use exceptions to indicate failures, I almost always use the “fail” keyword rather than the “raise” keyword in Ruby. Fail and raise are synonyms so there is no difference except that “fail” more clearly communicates that the method has failed. The only time I use “raise” is when I am catching an exception and re-raising it, because here I’m not failing, but explicitly and purposefully raising an exception. This is a stylistic issue I follow, but I doubt many other people do.
|
ACK |
ehelms
added
Not yet reviewed
Ready for merge
and removed
Not yet reviewed
Needs re-review
Needs testing
labels
fixes #8549 - Fixes subscription error messages for act key, BZ 1154619
No description provided.