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

Raise a notification for each repo that fails to enable #18290

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

carbonin
Copy link
Member

@carbonin
Copy link
Member Author

image

@miq-bot
Copy link
Member

miq-bot commented Dec 12, 2018

Checked commit carbonin@c25631a with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@@ -136,6 +136,7 @@ def enable_repos
LinuxAdmin::SubscriptionManager.enable_repo(repo, assemble_registration_options)
rescue AwesomeSpawn::CommandResultError
update_attributes(:upgrade_message => "failed to enable #{repo}")
Notification.create(:type => "enable_update_repo_failed", :options => {:repo_name => repo})
Copy link
Member

Choose a reason for hiding this comment

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

Looks good. I don't know if we can bubble up more information out of the exception but that might be too much information. Either way, that's more than what you need here.

@@ -285,6 +285,11 @@
:expires_in: 24.hours
:level: :error
:audience: superadmin
- :name: enable_update_repo_failed
:message: 'Subscription Manager failed to enable repo %{repo_name}'
Copy link
Member

Choose a reason for hiding this comment

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

I like how we're consistently inconsistent with putting periods at the end of the message. 🤣


it "raises a notification for repos which fail to enable" do
NotificationType.seed
result = AwesomeSpawn::CommandResult.new("stuff", "things", "more things", 1)
Copy link
Member

Choose a reason for hiding this comment

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

I approve of this command.

@jrafanie jrafanie merged commit 1eb52ea into ManageIQ:master Dec 12, 2018
@jrafanie jrafanie added this to the Sprint 101 Ending Dec 17, 2018 milestone Dec 12, 2018
simaishi pushed a commit that referenced this pull request Dec 13, 2018
Raise a notification for each repo that fails to enable

(cherry picked from commit 1eb52ea)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1518801
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit e5a0252945a13907ec76552daf1be194cb1f3759
Author: Joe Rafaniello <jrafanie@users.noreply.github.com>
Date:   Wed Dec 12 15:57:02 2018 -0500

    Merge pull request #18290 from carbonin/repo_enable_failure_notification
    
    Raise a notification for each repo that fails to enable
    
    (cherry picked from commit 1eb52eab66f61db2b207bbba60d19eca0826f0cd)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1518801

@carbonin carbonin deleted the repo_enable_failure_notification branch August 16, 2019 15:54
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.

4 participants